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

Convert all ap_verify test data to obs_test

    XMLWordPrintable

    Details

    • Story Points:
      4
    • Epic Link:
    • Sprint:
      AP S18-5, AP S18-6
    • Team:
      Alert Production

      Description

      Currently, running all unit tests in a single thread causes crashes due to conflicts between mapper initialization for obs_test (used by test_association and test_profiling) and obs_decam (used by test_dataset and test_ingestion). Once ap_verify is no longer specific to DECam, there is no longer a need for tests to use DECam data, and we should standardize on obs_test.

      This ticket must not be done as part of DM-13530, in order to ensure that the code is converted correctly.

        Attachments

          Issue Links

            Activity

            Hide
            mrawls Meredith Rawls added a comment -

            Pending a couple outstanding small comments over on GitHub, this looks fine. I think I'm convinced that running ap_verify in CI is going to achieve something closer to the "ideal test coverage" I'm concerned about.

            Show
            mrawls Meredith Rawls added a comment - Pending a couple outstanding small comments over on GitHub, this looks fine. I think I'm convinced that running ap_verify in CI is going to achieve something closer to the "ideal test coverage" I'm concerned about.
            Hide
            swinbank John Swinbank added a comment -

            Perhaps Eric Bellm or John Swinbank could comment on how frequently we're planning to run CI on ap_verify, but I'm hoping at least daily.

            I cannot give you a number on this, but yes, your conclusion is correct.

            More specifically: having science codes depend on the details of particular instrumentation is a layering violation. ap_verify (and ap_pipe, and effectively all our code which is not named obs_*) should aspire to be camera agnostic.

            Demonstrating that science codes operate correctly with particular instrumentation is practically the definition of an “integration test”!

            Show
            swinbank John Swinbank added a comment - Perhaps Eric Bellm or John Swinbank could comment on how frequently we're planning to run CI on ap_verify, but I'm hoping at least daily. I cannot give you a number on this, but yes, your conclusion is correct. More specifically: having science codes depend on the details of particular instrumentation is a layering violation. ap_verify (and ap_pipe, and effectively all our code which is not named obs_*) should aspire to be camera agnostic. Demonstrating that science codes operate correctly with particular instrumentation is practically the definition of an “integration test”!
            Hide
            krzys Krzysztof Findeisen added a comment -

            Oops, good point re: separation of responsibilities. I should have realized that myself. I'm getting sloppy...

            Show
            krzys Krzysztof Findeisen added a comment - Oops, good point re: separation of responsibilities. I should have realized that myself. I'm getting sloppy...
            Hide
            mrawls Meredith Rawls added a comment -

            Good clarification/reminder about unit tests vs. integration tests. As far as I'm concerned, you are welcome to merge this. The only bit that would be nice (but likely impractical, as discussed a bit at GitHub) is to avoid creating duplicates of the test files that live in obs_test

            Show
            mrawls Meredith Rawls added a comment - Good clarification/reminder about unit tests vs. integration tests. As far as I'm concerned, you are welcome to merge this. The only bit that would be nice (but likely impractical, as discussed a bit at GitHub) is to avoid creating duplicates of the test files that live in obs_test . 
            Hide
            krzys Krzysztof Findeisen added a comment -

            Thanks for the feedback, Meredith Rawls! Merged with changes.

            Show
            krzys Krzysztof Findeisen added a comment - Thanks for the feedback, Meredith Rawls ! Merged with changes.

              People

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

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.