Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-13567

Add tests to ap_pipe

    Details

    • Story Points:
      12
    • Epic Link:
    • Sprint:
      AP S19-3, AP S19-5, AP S19-6
    • Team:
      Alert Production

      Description

      The initial ap_pipe module had next to no tests. This ticket is to remedy that. The tests should try all the different possible ways to send input to ap_pipe and successfully run an image or two through each part of it.

        Attachments

          Issue Links

            Activity

            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            I'm not sure it's possible to make unit tests for ap_pipe; a single run-through of the entire pipeline on one CCD of one visit takes 68 seconds on my machine, so a unit test suite would probably be an order of magnitude beyond that. We may have to make do with fast integration tests instead.

            It might be possible to run unit tests with stub classes for ApPipeTask's subtasks, but I'm not sure it would be possible to run meaningful test assertions besides "was this task run on these dataIds?".

            Show
            krzys Krzysztof Findeisen added a comment - - edited I'm not sure it's possible to make unit tests for ap_pipe ; a single run-through of the entire pipeline on one CCD of one visit takes 68 seconds on my machine, so a unit test suite would probably be an order of magnitude beyond that. We may have to make do with fast integration tests instead. It might be possible to run unit tests with stub classes for ApPipeTask 's subtasks, but I'm not sure it would be possible to run meaningful test assertions besides "was this task run on these dataIds?".
            Hide
            mrawls Meredith Rawls added a comment -

            Krzysztof Findeisen and I pair coded on this today. We made an ap_pipe_testdata repo and are close to having a test that will test whether each subtask of ap_pipe was run once.

            Show
            mrawls Meredith Rawls added a comment - Krzysztof Findeisen and I pair coded on this today. We made an ap_pipe_testdata repo and are close to having a test that will test whether each subtask of ap_pipe was run once.
            Hide
            mrawls Meredith Rawls added a comment -

            We continued working on this yesterday. We have one working main test which uses mocks to verify that each main part of ap_pipe is run one time (processing, differencing, association, and forced photometry). We commented the code with a short list of tests we want to add next time to finish up this ticket.

            Show
            mrawls Meredith Rawls added a comment - We continued working on this yesterday. We have one working main test which uses mocks to verify that each main part of ap_pipe is run one time (processing, differencing, association, and forced photometry). We commented the code with a short list of tests we want to add next time to finish up this ticket.
            Hide
            mrawls Meredith Rawls added a comment -

            Would you please review these new tests, John Parejko? They were pair-coded by Krzysztof Findeisen and myself.

            Please note that the ticket includes a new ap_pipe_testdata GitHub repo and stack package. The developer guide does say that new packages "intended for distribution to end users" should be RFC'd; we have not done that as this is not really a user-facing package, but I can if you would like. It has already been added to lsst-dev in /project/shared/data/test_data per this community post.

            Show
            mrawls Meredith Rawls added a comment - Would you please review these new tests, John Parejko ? They were pair-coded by Krzysztof Findeisen and myself. Please note that the ticket includes a new ap_pipe_testdata GitHub repo and stack package. The developer guide does say that new packages "intended for distribution to end users" should be RFC'd; we have not done that as this is not really a user-facing package, but I can if you would like. It has already been added to lsst-dev in /project/shared/data/test_data per this community post .
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            I think regardless of whether we RFC ap_pipe_testdata, we need to make PRs in lsst/repos and lsst/lsstsw, as described in the new package guide. Otherwise, things will break when somebody tries to use ap_pipe with EUPS.

            Show
            krzys Krzysztof Findeisen added a comment - - edited I think regardless of whether we RFC ap_pipe_testdata , we need to make PRs in lsst/repos and lsst/lsstsw , as described in the new package guide . Otherwise, things will break when somebody tries to use ap_pipe with EUPS.
            Hide
            mrawls Meredith Rawls added a comment -

            Thank you for catching that Krzysztof Findeisen, I will add those now.

            Show
            mrawls Meredith Rawls added a comment - Thank you for catching that Krzysztof Findeisen , I will add those now.
            Hide
            Parejkoj John Parejko added a comment -

            Thanks for the cleanups, and for adding some tests of the pipeline!

            Show
            Parejkoj John Parejko added a comment - Thanks for the cleanups, and for adding some tests of the pipeline!

              People

              • Assignee:
                mrawls Meredith Rawls
                Reporter:
                mrawls Meredith Rawls
                Reviewers:
                John Parejko
                Watchers:
                John Parejko, Krzysztof Findeisen, Meredith Rawls
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: