Reorder PR and Mainline in Regression Test Runner #4720
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
kaukabrizvi
force-pushed
the
resumption_harness
branch
from
f309e6e to
7e9050b
Compare
dougch
approved these changes
Aug 20, 2024
jmayclin
approved these changes
Aug 20, 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.
Resolved issues:
This PR resolves an issue with the regression test failing in the rust bindings bump: https://github.com/aws/s2n-tls/actions/runs/10476995419/job/29017133119?pr=4719.
Description of changes:
Currently, the CI workflow runs performance tests on the pull request branch before running them on the mainline branch, which might lead to potential inconsistencies in the test results, especially when generating diffs. There was an issue with the version mismatch for the s2n-tls-sys dependency, where the required version 0.3.1 was not available, causing the build to fail. This was because the bindings are not regenerated after switching back to the PR branch from mainline. To avoid this issue from recurring on this bindings bump and future ones, this PR reorders the operations.
This PR addresses these issues by:
Reordering the workflow steps to first switch to the main branch, generate bindings, and run the performance tests on the mainline branch.
After completing the mainline tests, the workflow checks out the PR branch, regenerates bindings, and runs the performance tests on the PR branch.
Finally, it runs the differential performance test between the mainline and PR branch results.
Call-outs:
Instead of regenerating the bindings for the differential performance test, I reordered the mainline profiling and pull request profiling so that we only have to generate bindings once. The first git checkout step in the original workflow is used to ensure that the workflow starts by working with the code from the pull request branch. This is necessary because the GitHub Actions runner starts in a detached state, and the workflow needs to explicitly check out the pull request's code to work with it.
Testing:
How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?
Is this a refactor change? If so, how have you proved that the intended behavior hasn't changed?
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.