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

pytest runs extra tests in lsst.verify

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: verify
    • Labels:
      None

      Description

      When running pytest on a specific file in verify, it first runs all test files, then runs the named file a second time. This is not the expected behavior; in other packages the syntax pytest <file> runs only the tests in that file. Please fix this behavior so that only the requested tests are run, with no duplicates.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            I wonder if this is related to this line in the setup.cfg:

            addopts = --flake8 --doctest-modules python bin.src tests
            

            Show
            tjenness Tim Jenness added a comment - I wonder if this is related to this line in the setup.cfg : addopts = --flake8 --doctest-modules python bin.src tests
            Hide
            jsick Jonathan Sick added a comment -

            Yep. I double checked the docs and the module arguments aren't needed by the --doctest-modules flag. I think you go ahead and make this change, Krzysztof Findeisen. Do make sure that doctests are still being run.

            Show
            jsick Jonathan Sick added a comment - Yep. I double checked the docs and the module arguments aren't needed by the --doctest-modules flag. I think you go ahead and make this change, Krzysztof Findeisen . Do make sure that doctests are still being run.
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            I think I can see why the line is the way it is; the bug I reported seems to be the least of the various evils:

            setup.cfg scons pytest pytest <file>
            No --doctest-modules  Regular tests Regular tests <file> only
            --doctest-modules  Regular tests Error <file> only
            --doctest-modules python bin.src  Regular + doctests Doctests All doctests + <file>
             --doctest-modules python bin.src tests Regular + doctests, running regular tests twice  Regular + doctests   Regular + doctests, including <file> twice

            If I just provide --doctest-modules with no arguments, then pytest gives the following error for each file in bin.src:

            ../../lsstsw3/stack/Linux64/python_py/1.7.0/lib/python/py-1.7.0-py3.6.egg/py/_path/local.py:688: in pyimport
                raise self.ImportMismatchError(modname, modfile, self)
            E   py._path.local.LocalPath.ImportMismatchError: ('dispatch_verify', '[redacted]/devel/verify/bin/dispatch_verify.py', local('[redacted]/devel/verify/bin.src/dispatch_verify.py'))
            

            EDIT: updated to distinguish between scons and pytest

            Show
            krzys Krzysztof Findeisen added a comment - - edited I think I can see why the line is the way it is; the bug I reported seems to be the least of the various evils: setup.cfg scons pytest pytest <file> No --doctest-modules  Regular tests Regular tests <file> only --doctest-modules  Regular tests Error <file> only --doctest-modules python bin.src  Regular + doctests Doctests All doctests + <file>   --doctest-modules python bin.src tests Regular + doctests, running regular tests twice  Regular + doctests   Regular + doctests, including <file> twice If I just provide --doctest-modules with no arguments, then pytest gives the following error for each file in bin.src : ../../lsstsw3/stack/Linux64/python_py/1.7.0/lib/python/py-1.7.0-py3.6.egg/py/_path/local.py:688: in pyimport raise self.ImportMismatchError(modname, modfile, self) E py._path.local.LocalPath.ImportMismatchError: ('dispatch_verify', '[redacted]/devel/verify/bin/dispatch_verify.py', local('[redacted]/devel/verify/bin.src/dispatch_verify.py')) EDIT: updated to distinguish between scons and pytest
            Hide
            jsick Jonathan Sick added a comment -

            Talking with Tim Jenness, it seems that the best solution is to take --doctest-modules out of the setup.cfg for the reasons above and reintroduce doctests as a separate scons target (implemented in sconsUtils).

            Show
            jsick Jonathan Sick added a comment - Talking with Tim Jenness , it seems that the best solution is to take --doctest-modules out of the setup.cfg for the reasons above and reintroduce doctests as a separate scons target (implemented in sconsUtils ).
            Hide
            krzys Krzysztof Findeisen added a comment -

            Then I propose that Tim Jenness adopt this ticket, since my familiarity with sconsUtils is very limited.

            Show
            krzys Krzysztof Findeisen added a comment - Then I propose that Tim Jenness adopt this ticket, since my familiarity with sconsUtils is very limited.
            Hide
            jsick Jonathan Sick added a comment -

            See the discussion in the PR. Generally I want to move to pytest-doctestplus as a general-purpose solution to doctest testing (see also DM-18435), but I'd like an option on the best place to enable the pytest-doctestplus plugin: in individual package's setup.cfg files, or through sconsUtils?

            Show
            jsick Jonathan Sick added a comment - See the discussion in the PR . Generally I want to move to pytest-doctestplus as a general-purpose solution to doctest testing (see also DM-18435 ), but I'd like an option on the best place to enable the pytest-doctestplus plugin: in individual package's setup.cfg files, or through sconsUtils?
            Hide
            tjenness Tim Jenness added a comment -

            This looks like a good short term fix. We are trying to move to third party conda packages so for now I'd live with the package being part of conda and hope that by the time astropy removes it we will be using conda third parties more widely. The downside to adding it as a mandatory parameter to sconsUtils is that it doesn't seem to always work. I just tried the option in astro_metadata_translator and it breaks badly in the bin.src directory. The work around for that would be to have a switch in the tests/SConscript file to enable doctesting rather than always running it. If we are doing that we may as well be doing it in the setup.cfg file as we have here.

            Show
            tjenness Tim Jenness added a comment - This looks like a good short term fix. We are trying to move to third party conda packages so for now I'd live with the package being part of conda and hope that by the time astropy removes it we will be using conda third parties more widely. The downside to adding it as a mandatory parameter to sconsUtils is that it doesn't seem to always work. I just tried the option in astro_metadata_translator and it breaks badly in the bin.src directory. The work around for that would be to have a switch in the tests/SConscript file to enable doctesting rather than always running it. If we are doing that we may as well be doing it in the setup.cfg file as we have here.
            Hide
            jsick Jonathan Sick added a comment -

            Thanks. I'll keep working on the general solution in DM-18435 .

            Show
            jsick Jonathan Sick added a comment - Thanks. I'll keep working on the general solution in DM-18435 .

              People

              • Assignee:
                jsick Jonathan Sick
                Reporter:
                krzys Krzysztof Findeisen
                Reviewers:
                Tim Jenness
                Watchers:
                Jonathan Sick, Krzysztof Findeisen, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel