Split test CI job into test and test-journey
#1674
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This also stops installing `tree` on CI. The `tree` package provides the `tree` command, which is useful but does not currently appear to be used anywhere. At one time, various journey tests had used it to generate output for comparison, and this usage/dependency was declared with `with_program` in 688280b, which was also when the first command to install it was added to CI. Later, `tree` was found to be unportable or otherwise unsuitable in the counts it output, and all uses of it were replaced with `find` in 3c1bfd5 (GitoxideLabs#861). At that point, most `with_program` declarations for it were changed to `find`, but one of them in the `ein` journey tests was not replaced at that time. That test does use `find` (and not `tree`), and should declare its use of `find` rather than `tree`. This commit makes that change. One benefit of doing so is that it is possible to run the tests on a CI (or other) system without `tree` without skipping any tests, so this also removes it from the lists of packages to install with `apt-get` and `brew` in CI jobs.
In the `justfile`, this renames the old `ci-test` recipe to `ci-test-full`, and has `ci-test` no longer clean the target directory nor run journey tests. The CI `test` job thus remains the same, but it does moderately less work. A new CI job, `test-journey`, is introduced to run the journey tests (still via `ci-test-journey` recipe). This change is intended to allow greater parallelism, and possibly make caches work better. The CI `test` job has sometimes been a few minutes slower than before, ever since 5173e9a (GitoxideLabs#1668). See comments in GitoxideLabs#1668 for some discussion on this change.
Byron
approved these changes
Nov 14, 2024
There was a problem hiding this comment.
Thanks a lot, this is fantastic!
By the looks of it, the separation brings the CI runtime down to about 15 minutes, which is now dominated by the Windows jobs.
Once the journey-test cache kicks in, this particular job should also be faster, and I'd expect it to be faster than the 7 minutes I saw now.
In any case, now I'd expect no disk-space related issues to occur anymore, at least not for a while.
Edit: Merging, as follow-ups could be made in a separate PR.
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 14, 2024
This removes the `ci-test-full` justfile target (see GitoxideLabs#1674), and simplifies and shortens some comments in the justfile, since the distinction between `ci-test` and `ci-test-full` no longer has to be explained, and since some other parts of the comments are no longer applicable (GitoxideLabs#1673).
This was referenced Nov 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As detailed in 711c2f5, this splits out the journey tests into their own job, making changes to both
justfileand the CI test workflow to do so. This is the change recently discussed in #1668 (#1668 (comment), #1668 (comment)). I've tried to avoid making code comments in thejustfilemore confusing when doing this, but I may end up opening a subsequent PR to fix them up, depending on the answer to #1673.This also fixes up the remaining
with_program treein the journey tests, changing it towith_program find(f1a171b). All uses oftreein the journey tests were replaced withfindin 3c1bfd5 (#861), but this particularwith_programcall was not updated to reflect that. This allows the test to run whentreeis absent, and nothing else is using it, so this also no longer installs it in any CI jobs. The reason for including this change in this PR is that it was necessary to figure out which commands had to be run to prepare each oftestandtest-journey.