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

DM-45677: Remove MockApPipe.yaml and tests that need it #235

Merged
merged 3 commits into from
Sep 3, 2024

Conversation

kfindeisen
Copy link
Member

@kfindeisen kfindeisen commented Aug 26, 2024

This PR removes MockApPipe.yaml from our unit test system, replacing its use with more conventional (white-box) mocks.


  • Do unit tests pass (scons and/or stack-os-matrix)?
  • Did you run ap_verify.py on at least one of the standard datasets?
    For changes to metrics, the print_metricvalues script from lsst.verify will be useful.
  • Is the Sphinx documentation up-to-date?

Copy link
Contributor

@BrunoSanchez BrunoSanchez left a comment

Choose a reason for hiding this comment

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

I see a ton of code removed here. I think it is Ok.
If tests in CI passes then it is Ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this because the files that were removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

As stated in the commit message, no. This is an incidental problem I noticed while vetting ap_verify.table.

Copy link
Member Author

Choose a reason for hiding this comment

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

See https://github.com/lsst/templates/tree/main/project_templates/stack_package/example_pythononly for what a Python-only package is supposed to look like.

MockApPipe.yaml let us run black-box tests of pipeline execution, but
the pipeline was more prone to breakdowns than the pipeline driver
itself. Replace MockApPipe.yaml with a traditional mock, even though it
depends on implementation details of pipeline_driver.
ap_verify has always been a Python-only package, but when we first
created it we cargo-culted some C++ support infrastructure. Removing
these files will prevent future confusion.
@kfindeisen kfindeisen merged commit 0988827 into main Sep 3, 2024
2 checks passed
@kfindeisen kfindeisen deleted the tickets/DM-45677 branch September 3, 2024 19: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