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

Enable upstream Python tests. #45

Merged
merged 15 commits into from
Jul 20, 2023
Merged

Enable upstream Python tests. #45

merged 15 commits into from
Jul 20, 2023

Conversation

dumol
Copy link
Contributor

@dumol dumol commented Jun 20, 2023

Scope

Fixes #12

Changes

Enabled the Python tests on the platforms where we build it, with the exception of:

  • Alpine Linux
  • macOS.

Drive-by fixes:

  • fixed enabling EMUTRAMP on Alpine with paxctl.
  • fixed the execution of Shellcheck tests.
  • fixed minor Shellcheck warnings.
  • use dedicated chevabs functions for testing and installing to have them listed distinctly in GHA output.
  • enabled upstream tests for libffi, bzip2, xz, zlib.

Testing

Automated.

@dumol dumol self-assigned this Jun 20, 2023
@dumol
Copy link
Contributor Author

dumol commented Jun 23, 2023

Not sure what do about macOS, haven't invested much time into it. If you want to, take a look, maybe some things are worth reporting upstream.

Otherwise, I would just except macOS alongside Alpine Linux.

@dumol dumol mentioned this pull request Jul 6, 2023
@dumol dumol force-pushed the 12-python-upstream-tests branch from 70d70eb to 981dd02 Compare July 6, 2023 09:26
@adiroiban
Copy link
Member

it's ok to except macOS.

thanks

@dumol
Copy link
Contributor Author

dumol commented Jul 7, 2023

Sorry for the Shellcheck drive-by fixes, it looks like they weren't run at all in master because of a late "improvement" in the previous PR. :-/

I have now used execute for ./src/chevah-bash-tests/shellcheck_tests.sh, should not happen again…

One thing to keep in mind is that the only platform running all tests (upstream ones for OpenSSL/Python + ours for Python&modules + security check with safety + bash shell checks) is glibc-based Linux. You could argue Windows is also fully covered, as we use the upstream Python/OpenSSL/cryptography, so there's no need to test what upstream has built.

Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

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

I guess that it's ok.

I don't know where to check the result in GHA.

I was expecting to see it as a separate GHA step or at least as part of Pythia tests

image

My reading the code, this is mixed with compiling Python... which is a huge output, which is hard to check

My preference, is to have "Pythion test" as a separate top level step.

Thanks

# Don't use execute below, some tests might fail innocuously.
# execute "${MAKE_CMD[@]}" test

if [ "$OS" = "macos" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

can we have this as a separate command?

The command is "compile" , but this is not compile + test.

I think it's easier if we have a separate command.

And we can skip calling the test based on the GHA yaml files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added dedicated a chevahbs function for testing.

This testing is now listed distinctly in GHA output, alongside building and installing (installing can also be quite verbose, e.g. for OpenSSL and Python).

@dumol
Copy link
Contributor Author

dumol commented Jul 10, 2023

My reading the code, this is mixed with compiling Python... which is a huge output, which is hard to check

I've worked on separating testing in a dedicated GHA step. Same for installing.

My preference, is to have "Pythion test" as a separate top level step.

It is a separate test, but not a top-level one. I think this kind of testing is best done where it belongs, right after compiling, but before installing. This way, if testing fails, the build is stopped right there, no need to continue with building other libraries.

@adiroiban
Copy link
Member

I think that this can be merged. thanks!

@dumol dumol merged commit 5ac81e0 into master Jul 20, 2023
@dumol dumol deleted the 12-python-upstream-tests branch July 20, 2023 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable Python's own tests again.
3 participants