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

Fix failing tests on manylinux2014_i686 #912

Merged
merged 9 commits into from
Aug 16, 2024

Conversation

jesper-friis
Copy link
Collaborator

@jesper-friis jesper-friis commented Aug 11, 2024

Description

Fix failing tests on manylinux2014_i686.

Motivation: Fix the segfault we get on github for Python 3.12 on manylinux2014_i686. Unfortunately, I wasn't able to reproduce the segfault when running the docker locally.

While debugging, some issues related compiling on a 32 bit platform were revealed. These are fixed in this PR.

Fixes and changes in this PR:

  • Calculate instance hashes consistently across 32 and 64 bit systems. Since this anyway imposed a change in hashes, we used the opportunity to also include dimension and property descriptions in the hash calculations. This is a small change in behaviour. Since transactions haven't been used yet, this change shouldn't affect anyone.
  • Allow ctest to use PYTHONPATH to fine external packages that the tests depends on (like tripper, rdflib, ...).
  • Allow cmake to procede if hdf5 is not available. hdf5 support will in that case be turned off.
  • Compile with treads-support by default (since it is anyway used by Python which is enabled by default).
  • Set PATH and PYTHONPATH when running tests for Python scripts.

Unsolved issues:

  • Testing of wheels only runs the tests for the Python bindings by running python bindings/python/tests/test_python_bindings.py. Ideally we would like to run all tests using ctest, but that seems difficult to do... The reason is that ctest must be run from the build directory, but the wheel tests are defined by test-command in python/pyproject.toml. As far as I can see, there is no way to get the build directory from pyproject.toml.

Type of change

  • Bug fix & code cleanup
  • New feature
  • Documentation update
  • Test update

Checklist for the reviewer

This checklist should be used as a help for the reviewer.

  • Is the change limited to one issue?
  • Does this PR close the issue?
  • Is the code easy to read and understand?
  • Do all new feature have an accompanying new test?
  • Has the documentation been updated as necessary?

Copy link
Collaborator

@francescalb francescalb left a comment

Choose a reason for hiding this comment

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

Where is the change in the github workflows?

@jesper-friis
Copy link
Collaborator Author

Where is the change in the github workflows?

The failing tests were not run on GitHub CI. The wheel tests only runs a subset of the python tests. This should be fixed when the issue in tripper has been fixed. Should we do that in a new PR?

@jesper-friis
Copy link
Collaborator Author

TODO: Run all tests when building wheels, nut just a selection of the python tests

Copy link
Collaborator

@francescalb francescalb left a comment

Choose a reason for hiding this comment

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

looks OK

@jesper-friis jesper-friis merged commit 0d75273 into master Aug 16, 2024
14 checks passed
@jesper-friis jesper-friis deleted the compile-with-32bit-system branch August 16, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants