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

GitHub workflow for CI #1596

Merged
merged 1 commit into from
Jun 10, 2021
Merged

GitHub workflow for CI #1596

merged 1 commit into from
Jun 10, 2021

Conversation

ahoenselaar
Copy link
Contributor

@ahoenselaar ahoenselaar commented Jun 8, 2021

Scope:

  • C++ tests with MPI
  • C++ tests without MPI
  • Python v3.6 tests (MPI disabled) via pytest
  • Python v3.9 tests (MPI disabled) via pytest

This is intended as an initial set up step.

The failing Python tests are going to be addressed in follow-up PRs. Issues identified so far include:

  • Non-hermetic tests that break subsequent tests (verbosity_mgr)
  • Memory corruption in pympb that causes later tests to fail

Other improvements for follow-up PRs:

  • Cleaner presentation of test results
  • Automatic inclusion of a "unit test badge" in PRs

@ahoenselaar ahoenselaar marked this pull request as ready for review June 8, 2021 21:24
@oskooi
Copy link
Collaborator

oskooi commented Jun 9, 2021

When running make check on my local machine for this branch, python/tests/test_simulation.py is failing with the following error:

======================================================================
FAIL: test_at_time (__main__.TestSimulation)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./tests/test_simulation.py", line 142, in test_at_time
    self.assertTrue(os.path.exists(fname))
AssertionError: False is not true

======================================================================
FAIL: test_get_filename_prefix (__main__.TestSimulation)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./tests/test_simulation.py", line 532, in test_get_filename_prefix
    self.assertEqual(sim.get_filename_prefix(), "simulation")
AssertionError: 'test_simulation' != 'simulation'
- test_simulation
? -----
+ simulation


======================================================================
FAIL: test_use_output_directory_default (__main__.TestSimulation)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./tests/test_simulation.py", line 134, in test_use_output_directory_default
    self.assertTrue(os.path.exists(os.path.join(output_dir, self.fname)))
AssertionError: False is not true

======================================================================
FAIL: test_with_prefix (__main__.TestSimulation)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./tests/test_simulation.py", line 162, in test_with_prefix
    self.assertTrue(os.path.exists(fname))
AssertionError: False is not true

----------------------------------------------------------------------
Ran 32 tests in 40.139s

FAILED (failures=4)

configure.ac Outdated Show resolved Hide resolved
configure.ac Outdated Show resolved Hide resolved
configure.ac Outdated Show resolved Hide resolved
python/Makefile.am Outdated Show resolved Hide resolved
cp $(PY_PKG_FILES) meep
cp $(adjoint_PYTHON) meep/adjoint
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is going on with this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before this change, only the $PY_PKG_FILES were copied to the meep folder.
In a second step, the MPB interface was added below.
However, the adjoint subpackage was never copied to meep.

Do you have any insight why that is the case?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Our Python Makefile rules seem kind of hacky here; it would be better to rewrite them to cooperate with automake if possible.

@ahoenselaar
Copy link
Contributor Author

Example output: https://github.com/ahoenselaar/meep/runs/2796065822

Shows a few broken tests and a segfault due to memory corruption.
Tests after the verbosity manager test fail but that will be addressed by #1602.

@stevengj stevengj merged commit 94bb2f2 into NanoComp:master Jun 10, 2021
bencbartlett pushed a commit to bencbartlett/meep that referenced this pull request Sep 9, 2021
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.

3 participants