Skip to content

Split nav2_system_tests into separate CI jobs#3554

Merged
ruffsl merged 11 commits intomainfrom
ci-system-tests
May 9, 2023
Merged

Split nav2_system_tests into separate CI jobs#3554
ruffsl merged 11 commits intomainfrom
ci-system-tests

Conversation

@ruffsl
Copy link
Member

@ruffsl ruffsl commented May 3, 2023

to avoid CI timeout issues with CircleCI.

Related:

@ruffsl
Copy link
Member Author

ruffsl commented May 3, 2023

@SteveMacenski , would you happen see why this is listing no packages, rather than all excluding nav2_system_tests?

  BUILD_PACKAGES=$(
    colcon list \
      --names-only \
      --packages-select-regex * \
      --packages-skip-regex nav2_system_tests \
      --packages-above \
        $BUILD_UNFINISHED \
        $BUILD_FAILED \
        $BUILD_INVALID \
    | xargs)

https://app.circleci.com/pipelines/github/ros-planning/navigation2/9252/workflows/f47a38db-cabd-4c01-a6b3-b0855478d735/jobs/29837?invite=true#step-117-35

@ruffsl
Copy link
Member Author

ruffsl commented May 3, 2023

One caveat here is that we can either run the system_build job as soon as the release_build jobs succeeds, but then never cache the test results from system_test to avoid cache overwrites of test results from release_test.

OR

We can run the jobs linearly from release_build -> release_test -> system_build -> system_test, but this means that system_test may not get to run very often given how frequent tests in release_test fail. But perhaps that kind of fail fast and fail early is feature for the build_and_test workflow that is used for PRs. Not sure.

This is represented with before and after the commit: 95ff181 . Perhaps we could add some more conditional glue for reviewers to override this fail fast behavior, but it might take a few hacks or more refactoring:

@SteveMacenski
Copy link
Member

SteveMacenski commented May 3, 2023

We used to have a setup like

Debug build -> debug test

Release build -> release test

Where these were separate pipelines. At that time, we did store the test results of both because I'd look at them both before merging PRs. Could we do something like?

main_source_build -> system_tests_build -> main_source_test
                                        -> system tests_test

That would make it so that both sets of tests can run without if builds don't fail (which they never should). You mentioning caching problems, I don't know if things have changed since those older days that makes an issue now. I'm not concerned at all about caching the workspaces for system tests build since that always gets busted and runs no matter what (in case that makes things easier) - we only need the workspace cache for the source builds to speed things up. But I don't think that relates to the test results caching problem you mentioned

Is it not possible to have 2 results caches (or in different directories to separate?), one for each job? We don't need the cache for the system-tests-related workspaces, just the output files to see what tests failed and how.

I'm not sure if any of that is helpful to your question, maybe I'm just asking the same questions you asked yourself

ruffsl added 10 commits May 9, 2023 10:30
by skipping it in release build and test jobs
in order to skip them from system_test
by requiring release_test for system_build
to ensure linear cache store and restore access
to simplify catching of tests
given release_test was never timing out before
@ruffsl ruffsl force-pushed the ci-system-tests branch from 9c00424 to 1aa48e8 Compare May 9, 2023 08:30
to resolve package selection issues
@ruffsl
Copy link
Member Author

ruffsl commented May 9, 2023

I opted to keep the tests consolidated to one jobs and make it require the new system_build job instead, to simplify the caching of tests, given that the job wasn't timing out before.

@ruffsl ruffsl marked this pull request as ready for review May 9, 2023 10:32
@ruffsl ruffsl merged commit 49d0cdf into main May 9, 2023
@ruffsl ruffsl deleted the ci-system-tests branch May 9, 2023 10:33
@ruffsl ruffsl added the CI label May 9, 2023
jwallace42 pushed a commit to jwallace42/navigation2 that referenced this pull request May 9, 2023
* Refactor nav2_system_tests into Separate jobs
by skipping it in release build and test jobs

* Require release_test for system_build job
to linearly resume cache

* Keep cache test results from release_test
in order to skip them from system_test

* Alias matrix for system_build requires

* Refactor to use package select as well
to skip unrelated tests

* Use yaml anchors to keep DRY

* Rename regex parameters

* Retain cache for build_and_test workflows
by requiring release_test for system_build
to ensure linear cache store and restore access

* Bust cache to test CircleCI config

* Resume testing all packages from release_test
to simplify catching of tests
given release_test was never timing out before

* Remove parameter packages_select_regex
to resolve package selection issues
@ruffsl ruffsl mentioned this pull request May 10, 2023
ruffsl added a commit that referenced this pull request May 10, 2023
enricosutera pushed a commit to enricosutera/navigation2 that referenced this pull request May 19, 2024
* Refactor nav2_system_tests into Separate jobs
by skipping it in release build and test jobs

* Require release_test for system_build job
to linearly resume cache

* Keep cache test results from release_test
in order to skip them from system_test

* Alias matrix for system_build requires

* Refactor to use package select as well
to skip unrelated tests

* Use yaml anchors to keep DRY

* Rename regex parameters

* Retain cache for build_and_test workflows
by requiring release_test for system_build
to ensure linear cache store and restore access

* Bust cache to test CircleCI config

* Resume testing all packages from release_test
to simplify catching of tests
given release_test was never timing out before

* Remove parameter packages_select_regex
to resolve package selection issues

Signed-off-by: enricosutera <enricosutera@outlook.com>
enricosutera pushed a commit to enricosutera/navigation2 that referenced this pull request May 19, 2024
from: ros-navigation#3554
Signed-off-by: enricosutera <enricosutera@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants