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

Robustify tests/repository.py

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: daf_persistence
    • Labels:
      None
    • Team:
      External

      Description

      The TestParentRepository unit test can sometimes fail:

      price@master:/data3a/work/price/hscPipe5/lsstsw/build/daf_persistence/tests[(detached from 5.0-hsc)] $ python repository.py 
      .............E....
      ======================================================================
      ERROR: test (__main__.TestParentRepository)
      Tests 1. That an sqlite registry in a parent repo is used as the
      ----------------------------------------------------------------------
      Traceback (most recent call last):
        File "repository.py", line 790, in tearDown
          shutil.rmtree(self.testDir)
        File "/data3a/work/price/hscPipe5/lsstsw/miniconda/lib/python2.7/shutil.py", line 247, in rmtree
          rmtree(fullname, ignore_errors, onerror)
        File "/data3a/work/price/hscPipe5/lsstsw/miniconda/lib/python2.7/shutil.py", line 256, in rmtree
          onerror(os.rmdir, path, sys.exc_info())
        File "/data3a/work/price/hscPipe5/lsstsw/miniconda/lib/python2.7/shutil.py", line 254, in rmtree
          os.rmdir(path)
      OSError: [Errno 39] Directory not empty: '/data3a/work/price/hscPipe5/lsstsw/build/daf_persistence/tests/TestParentRepository/repoA'
       
      ----------------------------------------------------------------------
      Ran 18 tests in 0.695s
       
      FAILED (errors=1)
      

      I believe this is due to NFS dot-files hanging around, and doesn't reflect any real problem with the butler code.

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            Nate Pease, would you please review this?

            price@pap-laptop:~/LSST/daf_persistence (tickets/DM-10930=) $ git sub-patch
            commit a1383d6c42ad1bb46559ebc8b5806da4d78ed1a2
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Jun 14 18:23:38 2017 -0400
             
                test: robustify TestParentRepository
                
                The shutil.rmtree in the tearDown can fail (e.g., "Directory not empty"
                due to NFS dot files hanging around), which causes the whole test to fail.
                Allow it to fail. More important is that the directory not exist when the
                test starts, or more severe failures can occur that can delude the user
                into thinking something important is wrong.
             
            diff --git a/tests/repository.py b/tests/repository.py
            index c12d9c9..2478711 100644
            --- a/tests/repository.py
            +++ b/tests/repository.py
            @@ -784,10 +784,12 @@ class TestParentRepository(unittest.TestCase):
             
                 def setUp(self):
                     self.testDir = os.path.join(ROOT, 'TestParentRepository')
            +        if os.path.exists(self.testDir):
            +            shutil.rmtree(self.testDir)
             
                 def tearDown(self):
                     if os.path.exists(self.testDir):
            -            shutil.rmtree(self.testDir)
            +            shutil.rmtree(self.testDir, True)
             
                 def test(self):
             
            

            Show
            price Paul Price added a comment - Nate Pease , would you please review this? price@pap-laptop:~/LSST/daf_persistence (tickets/DM-10930=) $ git sub-patch commit a1383d6c42ad1bb46559ebc8b5806da4d78ed1a2 Author: Paul Price <price@astro.princeton.edu> Date: Wed Jun 14 18:23:38 2017 -0400   test: robustify TestParentRepository The shutil.rmtree in the tearDown can fail (e.g., "Directory not empty" due to NFS dot files hanging around), which causes the whole test to fail. Allow it to fail. More important is that the directory not exist when the test starts, or more severe failures can occur that can delude the user into thinking something important is wrong.   diff --git a/tests/repository.py b/tests/repository.py index c12d9c9..2478711 100644 --- a/tests/repository.py +++ b/tests/repository.py @@ -784,10 +784,12 @@ class TestParentRepository(unittest.TestCase): def setUp(self): self.testDir = os.path.join(ROOT, 'TestParentRepository') + if os.path.exists(self.testDir): + shutil.rmtree(self.testDir) def tearDown(self): if os.path.exists(self.testDir): - shutil.rmtree(self.testDir) + shutil.rmtree(self.testDir, True) def test(self):
            Hide
            price Paul Price added a comment -
            Show
            price Paul Price added a comment - Jenkins passed .
            Hide
            npease Nate Pease added a comment -

            looks good

            Show
            npease Nate Pease added a comment - looks good
            Hide
            price Paul Price added a comment -

            Thanks for the super-quick review, Nate! Merged to master.

            Show
            price Paul Price added a comment - Thanks for the super-quick review, Nate! Merged to master.

              People

              • Assignee:
                price Paul Price
                Reporter:
                price Paul Price
                Reviewers:
                Nate Pease
                Watchers:
                Nate Pease, Paul Price
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: