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

Disallow executable tests due to "#!/usr/bin/env python" problems on OSX

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Adopted
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      This topic came up during the pytest hacking session at the All Hands Meeting, but was not resolved there. Discussion about a recent code review brought it up again and Colin Slater reminded me that it was more dangerous than I'd thought.

      Because of the interaction between SIP and environment variables on macOS, making our tests executable with #!/usr/bin/env python and running them directly (./testBlah.py) will fail. Although some of our developers do use executable tests on Linux, I think we're better off disabling that entirely to prevent people trying to execute them on macOS. Also, that way everyone is executing the tests in the same manner and getting the same output via py.test.

      This will also help us in the future when we eventually start using py.test features (e..g fixtures and marks), directly, since then we'll have to run with py.test.

      We do have a lot of executable tests currently which would have to be changed and have their sha-bangs removed, but that's easily automated.

        Attachments

          Issue Links

            Activity

            Hide
            swinbank John Swinbank added a comment -

            As a curmudgeonly objector who missed my chance earlier, I feel I should point out that you can't adopt an RFC without creating the implementation tickets. In this case, I reckon that should at least include updating the Developer Guide (and I don't think you can legitimately attempt to enforce it until that's been done). I'd be sympathetic to the idea that updating all of the existing tests was outside scope.

            Show
            swinbank John Swinbank added a comment - As a curmudgeonly objector who missed my chance earlier, I feel I should point out that you can't adopt an RFC without creating the implementation tickets. In this case, I reckon that should at least include updating the Developer Guide (and I don't think you can legitimately attempt to enforce it until that's been done). I'd be sympathetic to the idea that updating all of the existing tests was outside scope.
            Hide
            tjenness Tim Jenness added a comment -

            John Swinbank is correct. An RFC can't be adopted if we don't know what work has been agreed to to declare it implemented.

            Show
            tjenness Tim Jenness added a comment - John Swinbank is correct. An RFC can't be adopted if we don't know what work has been agreed to to declare it implemented.
            Hide
            Parejkoj John Parejko added a comment -

            Oops! That was not intentional. I blame Simon Krughoff for telling me I can adopt it. I'll write a couple tickets.

            Updating existing tests seems like something that could either be done piecemeal when people feel like it, or done all in one go via an automated tool. I don't have an opinion between those options.

            Show
            Parejkoj John Parejko added a comment - Oops! That was not intentional. I blame Simon Krughoff for telling me I can adopt it. I'll write a couple tickets. Updating existing tests seems like something that could either be done piecemeal when people feel like it, or done all in one go via an automated tool. I don't have an opinion between those options.
            Hide
            tjenness Tim Jenness added a comment -

            I'm happy for it to be done any time people are editing some code in a package. The coding standard change is the important bit.

            Show
            tjenness Tim Jenness added a comment - I'm happy for it to be done any time people are editing some code in a package. The coding standard change is the important bit.
            Hide
            jsick Jonathan Sick added a comment -

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

            Developer Guide PR is still to-do.

            Show
            jsick Jonathan Sick added a comment - This Confluence page is tracking the migration: https://confluence.lsstcorp.org/pages/viewpage.action?pageId=58950873 Developer Guide PR is still to-do.

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              Parejkoj John Parejko
              Watchers:
              Colin Slater, Jim Bosch, John Parejko, John Swinbank, Jonathan Sick, Robert Lupton, Russell Owen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  CI Builds

                  No builds found.