Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-229

Python tests should be named test_*.py for pytest support

    XMLWordPrintable

Details

    • RFC
    • Status: Implemented
    • Resolution: Done
    • DM
    • None

    Description

      Now that we’re beginning to use pytest to run unit tests, it makes sense to use pytest’s default test discovery so that simply running py.test (without arguments) will have pytest discover and run all tests in the tests/ directory. Currently we must type py.test tests/*.py.

      We can achieve this simplicity through a coding standard that specifies all Python unit test modules must be named with the test_ pattern:

      tests/test_example.py
      

      (Our current standard is tests/testExample.py. The Python Style Guide implies this style, and the Python Unit Testing guide assumes this naming style.)

      I think this is a straightforward coding style change that will make life easier in the long run.

      Alternative

      An alternative to changing the names of test modules, while still making py.test work without arguments is to add a configuration file to each repo that tells pytest how to find our test modules.

      I don’t recommend this. I think it’s better just to use a correct and consistent naming schema.

      Attachments

        Issue Links

          Activity

            No builds found.
            jsick Jonathan Sick created issue -
            jsick Jonathan Sick made changes -
            Field Original Value New Value
            Link This issue relates to RFC-215 [ RFC-215 ]
            Parejkoj John Parejko added a comment -

            +1. Should this also trickle down to the test method names as well?

            Also, this implies that things that aren't tests should not begin with test.

            Parejkoj John Parejko added a comment - +1. Should this also trickle down to the test method names as well? Also, this implies that things that aren't tests should not begin with test .
            tjenness Tim Jenness added a comment -

            I think the test method names are already using the right form for test discovery (via unittest convention) so I dont' think this RFC needs to go so far as changing the content of every single file.

            I'm fine with a rename of the files, given that everyone already went and renamed a lot of them during the hack week. I think we are pretty safe assuming that all the files can be renamed as .py already works fine. I moved support code into sub directories in many cases to allow .py to work without bringing in support code but there might be a few left over.

            tjenness Tim Jenness added a comment - I think the test method names are already using the right form for test discovery (via unittest convention) so I dont' think this RFC needs to go so far as changing the content of every single file. I'm fine with a rename of the files, given that everyone already went and renamed a lot of them during the hack week. I think we are pretty safe assuming that all the files can be renamed as .py already works fine. I moved support code into sub directories in many cases to allow .py to work without bringing in support code but there might be a few left over.
            jsick Jonathan Sick added a comment - - edited

            Should this also trickle down to the test method names as well?

            In my experience, py.test will run the usual methods of a test class that inherits from unittest.TestCase. No need for change there (for pytest compatiblity).

            If/when we switch from unittest to native pytest functions, then yes, we’d want to use a test_ prefix for those functions.

            Also, this implies that things that aren't tests should not begin with test.

            I think pytest would just find the module, but not see any unittest.TestCase subclasses, so there should be no direct harm. I haven’t tested or thought deeply about this. But in principle, yes, non-test modules in a tests/ directory shouldn’t look like tests to humans either.

            jsick Jonathan Sick added a comment - - edited Should this also trickle down to the test method names as well? In my experience, py.test will run the usual methods of a test class that inherits from unittest.TestCase . No need for change there (for pytest compatiblity). If/when we switch from unittest to native pytest functions, then yes, we’d want to use a test_ prefix for those functions. Also, this implies that things that aren't tests should not begin with test. I think pytest would just find the module, but not see any unittest.TestCase subclasses, so there should be no direct harm. I haven’t tested or thought deeply about this. But in principle, yes, non-test modules in a tests/ directory shouldn’t look like tests to humans either.
            tjenness Tim Jenness added a comment -

            jsick is correct. pytest will read the file, find no tests and ignore it.

            tjenness Tim Jenness added a comment - jsick is correct. pytest will read the file, find no tests and ignore it.

            It would greatly simplify my life if this change is not made for anything below (and including) afw until the pybind11 port is complete.

            pschella Pim Schellart [X] (Inactive) added a comment - It would greatly simplify my life if this change is not made for anything below (and including) afw until the pybind11 port is complete.
            tjenness Tim Jenness added a comment -

            jsick Assuming the agreement is for a file rename with no change in content, I am happy for this RFC to be adopted so long as the implementation ticket can be correctly blocked by pschella's pybind11 work. pschella I assume it is okay to rename test files that are in packages that have no C++ code in them?

            tjenness Tim Jenness added a comment - jsick Assuming the agreement is for a file rename with no change in content, I am happy for this RFC to be adopted so long as the implementation ticket can be correctly blocked by pschella 's pybind11 work. pschella I assume it is okay to rename test files that are in packages that have no C++ code in them?

            tjenness, that is probably safe. But not completely, it may still be that these packages required some minor changes to the tests if they depended on changed lower level constructs.

            pschella Pim Schellart [X] (Inactive) added a comment - tjenness , that is probably safe. But not completely, it may still be that these packages required some minor changes to the tests if they depended on changed lower level constructs.
            jsick Jonathan Sick added a comment - - edited

            pschella, is there an epic I can properly block this RFC's possible implementation on?

            jsick Jonathan Sick added a comment - - edited pschella , is there an epic I can properly block this RFC's possible implementation on?

            I suppose you can block it on DM-8467.

            pschella Pim Schellart [X] (Inactive) added a comment - I suppose you can block it on DM-8467 .
            jsick Jonathan Sick made changes -
            Link This issue is blocked by DM-8467 [ DM-8467 ]

            I'm marking this as adopted as there are no concerns about the long-term value of this decision.

            There are two implementation options for this sort of change:

            1. Encourage developers to upgrade their packages as they go, or
            2. Combine this implementation with other codebase changes (RFC-107 and numpydoc).

            I think option 1 will work here since the change is so lightweight (a single git mv commit per repo). A good way to coordinate this will be with a wiki signup page, like we did for Python 3 and pytest.

            Finally, I confirm that implementation is blocked on pybind11 work (see linked ticket). The implementor should also confirm with pschella that the codebase is ready before going ahead.

            jsick Jonathan Sick added a comment - I'm marking this as adopted as there are no concerns about the long-term value of this decision. There are two implementation options for this sort of change: Encourage developers to upgrade their packages as they go, or Combine this implementation with other codebase changes ( RFC-107 and numpydoc). I think option 1 will work here since the change is so lightweight (a single git mv commit per repo). A good way to coordinate this will be with a wiki signup page, like we did for Python 3 and pytest. Finally, I confirm that implementation is blocked on pybind11 work (see linked ticket). The implementor should also confirm with pschella that the codebase is ready before going ahead.
            jsick Jonathan Sick made changes -
            Resolution Done [ 10000 ]
            Status Proposed [ 10805 ] Adopted [ 10806 ]
            jsick Jonathan Sick made changes -
            Link This issue is triggering DM-9024 [ DM-9024 ]
            tjenness Tim Jenness added a comment -

            Who is going to coordinate the work of renaming if we are going to adopt the python3 approach? Have you talked to the relevant T/CAMs about ensuring that their developers are aware of this change?

            tjenness Tim Jenness added a comment - Who is going to coordinate the work of renaming if we are going to adopt the python3 approach? Have you talked to the relevant T/CAMs about ensuring that their developers are aware of this change?
            tjenness Tim Jenness made changes -
            Link This issue relates to DM-7600 [ DM-7600 ]
            tjenness Tim Jenness made changes -
            Remote Link This issue links to "Page (Confluence)" [ 14907 ]
            jsick Jonathan Sick made changes -
            Remote Link This issue links to "Page (Confluence)" [ 14907 ]

            This Confluence page is tracking the migration: https://confluence.lsstcorp.org/pages/viewpage.action?pageId=58950873

            jsick Jonathan Sick added a comment - This Confluence page is tracking the migration: https://confluence.lsstcorp.org/pages/viewpage.action?pageId=58950873
            tjenness Tim Jenness made changes -
            Remote Link This issue links to "Page (Confluence)" [ 15080 ]
            tjenness Tim Jenness added a comment -

            jsick the work triggered by this RFC has been completed. Does this mean the RFC can be marked implemented? Are you waiting for the work on the confluence page to be completed (which I don't think is necessary, since this is a policy RFC).

            tjenness Tim Jenness added a comment - jsick the work triggered by this RFC has been completed. Does this mean the RFC can be marked implemented? Are you waiting for the work on the confluence page to be completed (which I don't think is necessary, since this is a policy RFC).

            That's true, I'll update the status to implemented.

            jsick Jonathan Sick added a comment - That's true, I'll update the status to implemented.
            jsick Jonathan Sick made changes -
            Status Adopted [ 10806 ] Implemented [ 11105 ]
            tjenness Tim Jenness made changes -
            Link This issue relates to RFC-370 [ RFC-370 ]

            People

              jsick Jonathan Sick
              jsick Jonathan Sick
              John Parejko, Jonathan Sick, Pim Schellart [X] (Inactive), Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                Jenkins

                  No builds found.