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

ci: update integ dependencies #4261

Merged
merged 5 commits into from
Nov 7, 2023
Merged

ci: update integ dependencies #4261

merged 5 commits into from
Nov 7, 2023

Conversation

jmayclin
Copy link
Contributor

Update CI dependencies used during integration testing

Description of changes:

pytest 5.3 was released 3 years ago, it is time to update dependencies.

This upgrade gives us access to the following features.

pytest -n auto

This would automatically set the number of test runners equal to the number of workers on the instance. However this can't be used because port collisions happen for not-well-understood reasons: #2127

pytest --durations=100

This prints out the 100 slowest tests in a given run. This give us some idea about how long the individual tests are taking to run, and helps us make sure that there aren't any outliers really increasing the runtime.

Call-outs:

271: 1.27s call     test_version_negotiation.py::test_s2nc_tls13_negotiates_tls12[S2N-OpenSSL-TLS1.2-RSA_4096_SHA512-P-521-DHE-RSA-AES128-SHA256]
--
271: 1.23s call     
test_version_negotiation.py::test_s2nc_tls13_negotiates_tls12[S2N-OpenSSL-TLS1.2-RSA_2048_SHA384-P-384-DHE-RSA-AES256-GCM-SHA384]

Tests were slower than I expected them to be, taking more than a second for each combinatorially exploded fixture configuration.

Testing:

All CI should pass

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Also print out the slowest tests for each run
@github-actions github-actions bot added the s2n-core team label Oct 24, 2023
Only print the 10 slowest tests, not the 100 slowest tests.
@jmayclin jmayclin marked this pull request as ready for review October 25, 2023 21:10
@jmayclin jmayclin requested a review from dougch as a code owner October 25, 2023 21:10
@jmayclin jmayclin requested a review from goatgoose October 25, 2023 21:11
@dougch
Copy link
Contributor

dougch commented Oct 25, 2023

The CMake version of the pytest command should probably match. #4030 is another attempt at tracking timing by using junit reports, again, only on newer versions of pytest.

@jmayclin
Copy link
Contributor Author

When not using Nix, the CMake version of the command still shells out to Tox and will use the same command.

In the Nix usecase, the testing commands have already diverged (e.g. Nix uses reruns set to 0 and doesn't use Tox at all).

My motivation for adding the durations command is that the integv2 job is currently the longest running mandatory CI job that we have and I want to make sure we aren't accidentally doing something silly, but that motivation doesn't quite apply to the Nix jobs.

Copy link
Contributor

@dougch dougch left a comment

Choose a reason for hiding this comment

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

We can do the nix update separately- this lays the groundwork for them to match.

@jmayclin jmayclin enabled auto-merge (squash) November 6, 2023 21:08
@jmayclin jmayclin merged commit db07a3d into aws:main Nov 7, 2023
@jmayclin jmayclin deleted the timing-integ branch June 15, 2024 00:16
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.

3 participants