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-23426: Use PipelineTask test framework for MetricTask #67

Merged
merged 7 commits into from Apr 2, 2020

Conversation

kfindeisen
Copy link
Member

This PR converts all MetricTask tests that previously used the custom code in tests/butler_utils.py to use the new PipelineTask test framework, and expands existing tests. I do not have a direct test of MetricTask.runQuantum, because of the difficulties of meaningfully testing the abstract MetricTask.

The test functions replace code previously in tests/butler_utils.py.
The test functions replace code previously in tests/butler_utils.py.
The daf.butler.tests and pipe.base.testUtils libraries provide the
same functionality.
Validation is not included in generic MetricTask tests both because
run may be an expensive method (it isn't for these MetricTasks) and
because validation needs good coverage of the possible task configs
to be meaningful.
Copy link

@morriscb morriscb left a comment

Choose a reason for hiding this comment

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

Looks fine to me. For my own reference, are the Butler calls you use here an example of what we should do when testing our own Gen3 PipelineTasks?

@kfindeisen
Copy link
Member Author

Maybe not "should" yet -- I'm still figuring out the best way to do things myself. For example, using the connections to call addDatasetType helps protect the tests from changes in MetricTask's many layers of inheritance, but is probably too awkward to use in general.

See the daf_butler and pipe_base documentation for this code, and note that I want to change the call to makeTestRepo in DM-24263 (though probably not soon).

@kfindeisen kfindeisen merged commit a3fec04 into master Apr 2, 2020
@kfindeisen kfindeisen deleted the tickets/DM-23426 branch April 2, 2020 18:30
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.

None yet

2 participants