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

test: only build requested unit tests in nix #4770

Merged
merged 8 commits into from
Sep 27, 2024
Merged

Conversation

lrstewart
Copy link
Contributor

Description of changes:

Fixes a couple usability issues with the nix develop script that were bugging me:

  • Functions should fail on the first error, not try to continue. Putting "-e" at the top of the script causes the whole shell to exit, but using subshells seems to work.
  • If I'm just trying to run one unit test, I don't want to build the resources needed by the integration tests. Especially not s2n_head.
  • If I'm just trying to run one unit test, I don't want to rebuild all the unit tests.
  • If I'm trying to run unit tests, I probably also want to recompile. It's a much rarer use case to just want to retry a test with no changes. And if that is the case, the recompile should basically be a no-op.

Testing:

I ran variations of the command locally. It seems to be able to handle wildcards / exact names / nothing just fine. The CI has a test of the nothing / all tests case.

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

nix/shell.sh Show resolved Hide resolved
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.

Makes sense; easier to iterate

@lrstewart lrstewart enabled auto-merge (squash) September 20, 2024 04:43
@lrstewart
Copy link
Contributor Author

lrstewart commented Sep 26, 2024

I shouldn't have just automatically stuck every command in a subshell 😅

  • "apache2_config" is only called by "apache2_start" to export a bunch of variables that "apache2_start" needs. Exporting those variables inside a subshell doesn't make them available to the parent, causing the nix integ tests to fail to build. I fixed that by removing the subshell for "apache2_config".
  • "apache2_start" is called by "integ" to start the apache server. But the server only lives as long as the subshell. I fixed that by removing the subshell for "apache2_start" too.

I also checked the other functions for similar problems: no other exports, and no other private / helper functions.

Out of paranoia, I also double-checked that exports are available in commands as expected:

[nix openssl-3.0] ubuntu@ip-172-31-28-113:~/s2n-tls$ export S2N_CMAKE_OPTIONS=notarealoption
[nix openssl-3.0] ubuntu@ip-172-31-28-113:~/s2n-tls$ echo $S2N_CMAKE_OPTIONS
notarealoption
[nix openssl-3.0] ubuntu@ip-172-31-28-113:~/s2n-tls$ configure
+---------------------------------------------------------+
| Configuring with cmake                                  |
+---------------------------------------------------------+
CMake Warning:
  Ignoring extra path from command line:

   "notarealoption"

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.

renegotiate_apache passes, LGTM

@lrstewart lrstewart merged commit 3807241 into aws:main Sep 27, 2024
37 checks passed
@lrstewart lrstewart deleted the nix branch September 27, 2024 05:56
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