Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Isolation of end-to-end tests #2047

Merged
merged 57 commits into from
May 12, 2024
Merged

Isolation of end-to-end tests #2047

merged 57 commits into from
May 12, 2024

Conversation

Adirelle
Copy link
Contributor

@Adirelle Adirelle commented May 10, 2024

This is an attempt to isolate the end-to-end tests from each others and from the configuration of the mise project itself.

It takes the following measures:

  • Create a temporary working tree for each test in the $TMPDIR / /tmp folder. This folder is removed if the test succeeds.
  • Install the dummy plugin from the source tree in the working tree. This allows to do quick and offline tests.
  • Get rid of the shared fixtures files. Each test creates the required files in its script.
  • Enforce using e2e/run_test to run the tests. They are not executable anymore to prevent running them in the user HOME.
  • Use env --ignore-environment ... to tightly control the environment variables. This includes overriding $PATH with a default value.
  • Move the test scripts around to mimics the src file tree.
  • Rename slow tests using _slow. They are skipped by run_all_tests unless TEST_ALL=1.
  • Check that the required commands for a test is available in the (restricted) PATH or skip the test with an explicit message. This should not happen in the dev container.
  • Print both successful and failed assertions, with colors if possible. Use Github Action annotations in CI.

Note: this will require tests on MacOS.

e2e/assert.sh Outdated Show resolved Hide resolved
@Adirelle Adirelle force-pushed the test-isolation branch 3 times, most recently from 675e582 to 7e4c57c Compare May 10, 2024 17:08
@Adirelle
Copy link
Contributor Author

Adirelle commented May 11, 2024

In these tests, different commands are used to test if a binary is available in PATH:

  • which $CMD => which is a POSIX-compliant binary
  • type -p $CMD => type is a bash builtin
  • command $CMD => command seems to be a fish builtin but I am not sure.

Which one would work on both MacOS and Linux?

@Adirelle
Copy link
Contributor Author

Some of the plugins (and tests) require some tools to be available in the PATH, e.g. the pipx forge requires pipx to be in the PATH.
According to some testing, it does not support reusing pipx installed by another plugin.

In the e2e tests, what should be done?

  1. install the required tool(s) in the test,
  2. add the tools in the docker test image and skip the test if the command is not available,
  3. or another option?

@Adirelle
Copy link
Contributor Author

The ubi test is failing because the test expects ubi to be installed in the dev container, which is not the case right yet.

@Adirelle Adirelle force-pushed the test-isolation branch 2 times, most recently from f2f00c9 to 298397b Compare May 12, 2024 12:21
@jdx
Copy link
Owner

jdx commented May 12, 2024

Some of the plugins (and tests) require some tools to be available in the PATH, e.g. the pipx forge requires pipx to be in the PATH.
According to some testing, it does not support reusing pipx installed by another plugin.

maybe try testing on the latest changes since I should have fixed this bug yesterday

@jdx
Copy link
Owner

jdx commented May 12, 2024

I think bootstrapping with mise would be the best approach generally if we can get it working, backends are definitely still a bit wonky though so we'll just do the best we can

Copy link

codacy-production bot commented May 12, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.18%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (76202de) 16867 13752 81.53%
Head commit (f043512) 16867 (+0) 13722 (-30) 81.35% (-0.18%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#2047) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy will stop sending the deprecated coverage status from June 5th, 2024. Learn more

source "$(dirname "$0")/assert.sh"
# shellcheck shell=bash

cat >.mise.toml <<CONF
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is such a tiny nit I don't even want you to change the current code, but in the future I prefer sticking to EOF for all here-docs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes. I wrote this at the very beginning then I switched to EOF but forgot this one.

@Adirelle Adirelle marked this pull request as ready for review May 12, 2024 15:47
This prevents cargo-binstall from hitting the API rate limit if GITHUB.
The argument is not persisted in the image; it is only available at build time.
There are still slow, but at least they pass in the dev container.
@jdx jdx enabled auto-merge May 12, 2024 15:49
@jdx jdx disabled auto-merge May 12, 2024 16:17
@jdx jdx merged commit ebe4917 into jdx:main May 12, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants