search

bluesky / event-model

data model for event-based data collection and analysis
https://blueskyproject.io/event-model
BSD 3-Clause "New" or "Revised" License
15 stars 31 forks source link

Fix RunRouter subfactory bug #171

Closed jklynch closed 4 years ago

jklynch commented 4 years ago

This PR addresses #170.

Description

Added field self._start_to_start_doc to RunRouter. In RunRouter.start(...) store the start document by uid. In RunRouter.descriptor(...) retrieve the associated start document and give it to subfactory callbacks. In RunRouter.stop(...) remove the start document from self._start_to_start_doc.

How Has This Been Tested?

The test function test_subfactory() in test_run_router.py was written to target the bug reported in #170.

The test function test_run_router() has been moved from test_em.py to test_run_router.py to keep RunRouter tests together.

jklynch commented 4 years ago

The initial commit adds a test to demonstrate the bug, but the test may not be quite right yet. At least it fails!

danielballan commented 4 years ago

An elegant test.

I agree we are overdue for splitting up the event-model tests across more than more file. Would you mind moving this embarrassingly monolithic "unit test" into the new file as part of this PR, so we can keep the RunRouter tests together?

danielballan commented 4 years ago

I confirmed that running the test in its final (fixed) form against the master branch version of event-model, I can reproduce the expected bug:

>       assert subfactory_documents["start"] == [run_bundle.start_doc]
E       AssertionError: assert [{'configurat...rimary', ...}] == [{'time': 158...742a544daa0'}]