Add more minimal CI test
This change adds one more build to the CI/CD chain to test for the Boost dependency.
It seems difficult to uninstall the boost headers due to dependencies of other packages. Important would be a proof that the CI fails when #3258 is reverted. Do you have an idea how to best go about that? :thinking:
Pull request automatically marked stale!
This seems good to go once the break is reverted. With the commit https://github.com/nest/nest-simulator/pull/3257/commits/243449d35f5c154a90fcf54e5441380d94d34a94 correctly being flagged broken by the CI:
Error: /home/runner/work/nest-simulator/nest-simulator/libnestutil/iterator_pair.h:26:10: fatal error: boost/iterator/iterator_adaptor.hpp: No such file or directory
26 | #include <boost/iterator/iterator_adaptor.hpp>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
:warning: This is however only partial protection as we have seen in https://github.com/nest/nest-simulator/pull/3257/commits/ff5dbc3161c8c892b5e5553d5b9f500e5ca6c516 not being flagged. This additional build can only protect against boost-includes in parts of the code that is not include-guarded by a combination of other dependencies (such as GSL in this case). To protect against this we would need to duplicate the whole test matrix. :zap:
@gtrensch, @lekshmideepu, could you have another look?
Pull request automatically marked stale!
Improved CI runs additionally revealed a spurious call to mpirun in case of -Dwith-mpi=OFF somewhere in the pytest suite (now becoming visible since mpi is not installed anymore on the runner):
________________ ERROR at setup of test_pairwise_bernoulli[12] _________________
[gw0] linux -- Python 3.10.18 /opt/hostedtoolcache/Python/3.10.18/x64/bin/python3.10
@pytest.fixture(scope="session")
def subprocess_compatible_mpi():
"""Until at least OpenMPI 4.1.6, the following fails due to a bug in OpenMPI, from 5.0.7 is definitely safe."""
> res = subprocess.run(["mpirun", "-np", "1", "echo"])
@gtrensch, the original boost issue is now proven to be caught. I updated the description above. The two commits also show again, that multiple #ifdef may hide the bug from our small build matrix.
The erroneously failing MPI test during non-MPI builds still needs to be fixed.
Thanks for your comments and discussion on this. It helped and improved our understanding of the matter a lot.
@terhorstd, all builds and checks have passed! Depending on CMake option '-Dwith-mpi=on/off' and the availability of 'mpi4py', MPI tests are executed or skipped correctly.
Wow, thanks @gtrensch for fixing this :clap:
I agree too your comment that most of the try…except…if… parts are problematic. I would have expected the @unittest.skipIf(not HAVE_MPI, "NEST was compiled without MPI") solution would work in most places, but I see that it would need some deeper work to get that sorted out. I'd opt for putting this in for now and opening a separate issue for looking into a better solution for the HAVE_MPI in the test suite.
I cannot assign myself as reviewer, but agree :+1: to your changes to fix the downstream matter. They are quite orthogonal, so @gtrensch, for the remaining changes should be unproblematic for you to stay reviewer for this PR. As soon as we have both thumbs up again we can merge.
@terhorstd, many thanks for your kind comments. I agree completely. The test suite should handle this at a higher level.
@lekshmideepu Ping!
Thanks for the very enlightening process and inspiring reviews :+1: