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-26947: generalize test support code and add test for Butler.get on calibrations #384

Merged
merged 2 commits into from Sep 29, 2020

Conversation

TallJimbo
Copy link
Member

@TallJimbo TallJimbo commented Sep 26, 2020

I forgot to ask for a review of 2062a8d when I added it to DM-26629 after the main review for that ticket was complete. This branch address two obvious problems with that commit - lack of docs and tests - already, but I'd like to have that commit reviewed here as well as the code actually on this branch.

helper._finish()
def makeButler(self, **kwargs: Any) -> Butler:
config = ButlerConfig()
config["registry", "db"] = "sqlite://"
Copy link
Member

Choose a reason for hiding this comment

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

It might be clearer here if you use :memory: explicitly here like is done in https://github.com/lsst/daf_butler/blob/master/python/lsst/daf/butler/tests/_testRepo.py#L76

This renames test_transfers.py to test_simpleButler.py, and modifies
its test infrastructure to create and use Butler objects instead of
duplicating some of Butler's logic.

I think this will be a better place than test_butler.py to put some
other new butler tests (and maybe some old ones, too) for a few
reasons:

 - it provides a way to avoid combinatorial testing of butler
 configurations when that isn't justified;

 - it tests the default configuration of butler, not the test
 configurations;

 - it provides a way to use the tests/data/registry yaml export files
 to set up test data.
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