arkade icon indicating copy to clipboard operation
arkade copied to clipboard

Improve e2e tests to cover more OS and architectures

Open Jasstkn opened this issue 1 year ago • 7 comments

I use mac with arm architecture on daily basis and it's tiresome to fix all broken links for mac and/or arm architecture.

Probably related to #557

Expected Behaviour

Tool that have mac and/or arm builds successfully installed.

Current Behaviour

Very often I receive 404.

Are you a GitHub Sponsor yet (Yes/No?)

  • [ ] Yes
  • [x] No

Possible Solution

  1. create a workflow to detect all broken links
  2. fix all that possible
  3. define threshold of allowed number to fail
  4. schedule workflow to run every N-days (like every week)

Steps to Reproduce (for bugs)

  1. N/A

Context

I changed architecture and os for e2e tests, here are results:

# os := "darwin"
# arch := "amd64"
# total 40
    --- FAIL: Test_CheckTools/Download_of_goreleaser (0.68s)
    --- FAIL: Test_CheckTools/Download_of_terrascan (0.78s)
    --- FAIL: Test_CheckTools/Download_of_sops (0.74s)
    --- FAIL: Test_CheckTools/Download_of_mkcert (0.66s)
    --- FAIL: Test_CheckTools/Download_of_k0s (0.91s)
    --- FAIL: Test_CheckTools/Download_of_kubecm (0.63s)
    --- FAIL: Test_CheckTools/Download_of_hostctl (0.62s)
    --- FAIL: Test_CheckTools/Download_of_argocd-autopilot (0.62s)
    --- FAIL: Test_CheckTools/Download_of_hadolint (0.64s)
    --- FAIL: Test_CheckTools/Download_of_cr (0.62s)
    --- FAIL: Test_CheckTools/Download_of_rpk (0.60s)
    --- FAIL: Test_CheckTools/Download_of_vault (0.72s)
    --- FAIL: Test_CheckTools/Download_of_bun (0.64s)
    --- FAIL: Test_CheckTools/Download_of_tkn (0.64s)
    --- FAIL: Test_CheckTools/Download_of_nerdctl (0.70s)
    --- FAIL: Test_CheckTools/Download_of_argocd (0.68s)
    --- FAIL: Test_CheckTools/Download_of_promtool (0.62s)
    --- FAIL: Test_CheckTools/Download_of_docker-compose (0.75s)
    --- FAIL: Test_CheckTools/Download_of_hubble (0.60s)
    --- FAIL: Test_CheckTools/Download_of_hugo (0.63s)
    --- FAIL: Test_CheckTools/Download_of_kube-bench (0.63s)
    --- FAIL: Test_CheckTools/Download_of_kail (0.59s)
    --- FAIL: Test_CheckTools/Download_of_stern (0.61s)
    --- FAIL: Test_CheckTools/Download_of_krew (0.82s)
    --- FAIL: Test_CheckTools/Download_of_waypoint (0.65s)
    --- FAIL: Test_CheckTools/Download_of_packer (0.68s)
    --- FAIL: Test_CheckTools/Download_of_vagrant (0.67s)
    --- FAIL: Test_CheckTools/Download_of_terraform (0.65s)
    --- FAIL: Test_CheckTools/Download_of_doctl (0.24s)
    --- FAIL: Test_CheckTools/Download_of_popeye (0.60s)
    --- FAIL: Test_CheckTools/Download_of_kubebuilder (0.28s)
    --- FAIL: Test_CheckTools/Download_of_k9s (0.62s)
    --- FAIL: Test_CheckTools/Download_of_eksctl (0.63s)
    --- FAIL: Test_CheckTools/Download_of_kubeseal (0.69s)
    --- FAIL: Test_CheckTools/Download_of_cilium (0.64s)
    --- FAIL: Test_CheckTools/Download_of_nats-server (0.64s)
    --- FAIL: Test_CheckTools/Download_of_k3s (0.67s)
    --- FAIL: Test_CheckTools/Download_of_kubectl (0.42s)
    --- FAIL: Test_CheckTools/Download_of_jq (0.75s)
    --- FAIL: Test_CheckTools/Download_of_helmfile (0.70s)

# os := "darwin"
# arch := "arm64"
# total 25
--- FAIL: Test_CheckTools/Download_of_doctl (0.23s)
--- FAIL: Test_CheckTools/Download_of_kubebuilder (0.08s)
--- FAIL: Test_CheckTools/Download_of_nerdctl (0.31s)
--- FAIL: Test_CheckTools/Download_of_tkn (0.65s)
--- FAIL: Test_CheckTools/Download_of_k3s (0.67s)
--- FAIL: Test_CheckTools/Download_of_argocd (0.45s)
--- FAIL: Test_CheckTools/Download_of_helmfile (0.66s)
--- FAIL: Test_CheckTools/Download_of_docker-compose (0.68s)
--- FAIL: Test_CheckTools/Download_of_hugo (0.63s)
--- FAIL: Test_CheckTools/Download_of_kail (0.28s)
--- FAIL: Test_CheckTools/Download_of_kube-bench (0.63s)
--- FAIL: Test_CheckTools/Download_of_hadolint (0.51s)
--- FAIL: Test_CheckTools/Download_of_krew (0.46s)
--- FAIL: Test_CheckTools/Download_of_stern (0.62s)
--- FAIL: Test_CheckTools/Download_of_gh (0.45s)
--- FAIL: Test_CheckTools/Download_of_waypoint (0.73s)
--- FAIL: Test_CheckTools/Download_of_packer (0.73s)
--- FAIL: Test_CheckTools/Download_of_vagrant (0.76s)
--- FAIL: Test_CheckTools/Download_of_goreleaser (0.29s)
--- FAIL: Test_CheckTools/Download_of_k0s (0.45s)
--- FAIL: Test_CheckTools/Download_of_argocd-autopilot (0.46s)
--- FAIL: Test_CheckTools/Download_of_influx (0.53s)
--- FAIL: Test_CheckTools/Download_of_kubecm (0.46s)
--- FAIL: Test_CheckTools/Download_of_mkcert (0.45s)
--- FAIL: Test_CheckTools/Download_of_hostctl (0.42s)

Your Environment

  • What Kubernetes distribution are you using?
Client Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.0", GitCommit:"a866cbe2e5bbaa01cfd5e969aa3e033f3282a8a2", GitTreeState:"clean", BuildDate:"2022-08-23T17:36:43Z", GoVersion:"go1.19", Compiler:"gc", Platform:"darwin/arm64"}
Kustomize Version: v4.5.7
  • Operating System and version (e.g. Linux, Windows, MacOS):
Darwin mariakot-osx 21.6.0 Darwin Kernel Version 21.6.0: Wed Aug 10 14:28:35 PDT 2022; root:xnu-8020.141.5~2/RELEASE_ARM64_T8101 arm64
  • What arkade version is this?
Version: 0.8.39
Git Commit: 2e76c2c3681a3796b03483c3c04fb2ebcba0fa3e

Jasstkn avatar Sep 02 '22 15:09 Jasstkn

@Shikachuu Hello! If you have time, please, provide your feedback about this issue. I think you might bring valuable input 🙏🏻

Jasstkn avatar Sep 02 '22 15:09 Jasstkn

Hey @Jasstkn,

The idea and the proposal theoritically looks simple, technically I think it isn't. Right now, we only provide information about the binary in the templates (BinaryTemplate and URLTemplate), so currently we have no way determine if a given binary supports a given arch + OS combo. Which leads to my first point:

  1. We can't separate valid alerts from false alerts in these tests.
  2. While there is nothing wrong with purposely always failing tests / ignoring failing tests results, it can potentially confuses current and future contributors why the test are failing / designed this way and they will distrust automated tests and quality checks, which leads to a decrasing code quality overall, but thats just my experience.

Perhaps we can add a new field to the Tool struct where we can define a slice of arch + os combos. That could solve a lot of issues, like provide a nicer way to say, hey your os or arch is not supported by this tool instead of a 404, provide better manual and e2e testability, cause we know what the output should look like, and ease in the review process, since we can pinpoint missing test without the need of checkingt the tool's documentation. While it has a lot of benefits, it is a huge refactor, we need to create this slices for all the existing tools, where we'll potentially encounter missing os-arch paris or missing tests.

But thats just my take on the issue and there is a chance that I missed something. @alexellis what's your take on this?

Shikachuu avatar Sep 02 '22 18:09 Shikachuu

Hey @Jasstkn,

The idea and the proposal theoritically looks simple, technically I think it isn't. Right now, we only provide information about the binary in the templates (BinaryTemplate and URLTemplate), so currently we have no way determine if a given binary supports a given arch + OS combo. Which leads to my first point:

  1. We can't separate valid alerts from false alerts in these tests.
  2. While there is nothing wrong with purposely always failing tests / ignoring failing tests results, it can potentially confuses current and future contributors why the test are failing / designed this way and they will distrust automated tests and quality checks, which leads to a decrasing code quality overall, but thats just my experience.

Perhaps we can add a new field to the Tool struct where we can define a slice of arch + os combos. That could solve a lot of issues, like provide a nicer way to say, hey your os or arch is not supported by this tool instead of a 404, provide better manual and e2e testability, cause we know what the output should look like, and ease in the review process, since we can pinpoint missing test without the need of checkingt the tool's documentation. While it has a lot of benefits, it is a huge refactor, we need to create this slices for all the existing tools, where we'll potentially encounter missing os-arch paris or missing tests.

But thats just my take on the issue and there is a chance that I missed something. @alexellis what's your take on this?

Yes, that's why I was thinking about scheduled run instead of running for every PR. It will be more like reporting system.

Regarding code changes - it seems to be a good approach but we need to carefully think through it.

Jasstkn avatar Sep 03 '22 09:09 Jasstkn

I also use an M1. Unfortunately many projects do not ship binaries for us. Arkade was started 3 years ago and it's possible that some projects have progressed since then. PRs are welcome.

When that's the case or arkade hasn't been updated yet, you can use the --arch x86_64 flag. I did that this morning for stern.

alexellis avatar Sep 04 '22 18:09 alexellis

@Jasstkn I think we can do more, would you like to lead the effort?

Perhaps we could generate some kind of markdown table, to inform what CLIs may be missing and for which architectures?

We would use it for our own purposes, to potentially raise issues on the projects that don't support ARM M1 yet, and to prioritise which templates to edit in arkade for things that are popular and useful to us personally like stern.

WDYT?

alexellis avatar Sep 05 '22 08:09 alexellis

@Jasstkn I think we can do more, would you like to lead the effort?

Perhaps we could generate some kind of markdown table, to inform what CLIs may be missing and for which architectures?

We would use it for our own purposes, to potentially raise issues on the projects that don't support ARM M1 yet, and to prioritise which templates to edit in arkade for things that are popular and useful to us personally like stern.

WDYT?

Yes, I would like to implement this. Let me take few days to work on it and I'll create a draft PR once something is ready.

Jasstkn avatar Sep 05 '22 13:09 Jasstkn

/assign: me

Jasstkn avatar Sep 08 '22 19:09 Jasstkn

I've tried to implement this but the unexpected issue was hitting to Github's limits. Therefore I think this approach is bad idea in general.

Jasstkn avatar Oct 15 '22 18:10 Jasstkn