Merge pull request #3393 from vkarak/bugfix/dont-restage-skip-clone

[bugfix] Do not copy or `git clone` `sourcesdir` if `--dont-restage` is passed

Author: vkarak

docs/manpage.rst

@@ -477,6 +477,10 @@ Options controlling ReFrame output
 
    .. versionadded:: 3.1
 
+   .. warning::
+
+      Running a test with :option:`--dont-restage` on a stage directory that was created with a different ReFrame version is undefined behaviour.
+
 .. option:: --keep-stage-files
 
    Keep test stage directories even for tests that finish successfully.

reframe/core/pipeline.py

@@ -20,6 +20,7 @@
 import numbers
 import os
 import shutil
+from pathlib import Path
 
 import reframe.core.fields as fields
 import reframe.core.hooks as hooks
@@ -1779,6 +1780,21 @@ def setup(self, partition, environ, **job_opts):
         self._setup_container_platform()
         self._resolve_fixtures()
 
+    def _requires_stagedir_contents(self):
+        '''Return true if the contents of the stagedir need to be generated'''
+
+        # Every time the stage directory is created a fresh mark is created.
+        # Normally, this is wiped out before running the test, unless
+        # `--dont-restage` is passed. In this case, we want to leave the
+        # existing stagedir untouched.
+
+        mark = Path(self.stagedir) / '.rfm_mark'
+        if mark.exists():
+            return False
+        else:
+            mark.touch()
+            return True
+
     def _copy_to_stagedir(self, path):
         self.logger.debug(f'Copying {path} to stage directory')
         self.logger.debug(f'Symlinking files: {self.readonly_files}')
@@ -1832,11 +1848,12 @@ def compile(self):
                     f'interpreted as relative to it'
                 )
 
-            if osext.is_url(self.sourcesdir):
-                self._clone_to_stagedir(self.sourcesdir)
-            else:
-                self._copy_to_stagedir(os.path.join(self._prefix,
-                                                    self.sourcesdir))
+            if self._requires_stagedir_contents():
+                if osext.is_url(self.sourcesdir):
+                    self._clone_to_stagedir(self.sourcesdir)
+                else:
+                    self._copy_to_stagedir(os.path.join(self._prefix,
+                                                        self.sourcesdir))
 
         # Set executable (only if hasn't been provided)
         if not hasattr(self, 'executable'):
@@ -2628,7 +2645,7 @@ def run(self):
         The resources of the test are copied to the stage directory and the
         rest of execution is delegated to the :func:`RegressionTest.run()`.
         '''
-        if self.sourcesdir:
+        if self.sourcesdir and self._requires_stagedir_contents():
             if osext.is_url(self.sourcesdir):
                 self._clone_to_stagedir(self.sourcesdir)
             else:

unittests/test_cli.py

@@ -339,23 +339,42 @@ def test_check_sanity_failure(run_reframe, tmp_path, run_action):
 
 
 def test_dont_restage(run_reframe, tmp_path):
-    run_reframe(
-        checkpath=['unittests/resources/checks/frontend_checks.py'],
-        more_options=['-n', 'SanityFailureCheck']
-    )
+    # Here we test four properties of `--dont-restage`
+    # 1. If the stage directory has not been populated before, it will
+    # 2. If the stage directory has been populated, it will stay untouched
+    # 3. The sourcesdir will not be copied again
+    # 4. When combined with `--max-retries`, the stage directory is reused
+
+    returncode = run_reframe(
+        checkpath=['unittests/resources/checks/'],
+        more_options=['-n', 'SanityFailureCheck', '-n', '^HelloTest$',
+                      '--dont-restage', '--keep-stage-files']
+    )[0]
+    # Assert property (1)
+    assert returncode != 0
 
-    # Place a random file in the test's stage directory and rerun with
-    # `--dont-restage` and `--max-retries`
+    stagedir_runonly = (tmp_path / 'stage' / 'generic' / 'default' /
+                        'builtin' / 'SanityFailureCheck')
     stagedir = (tmp_path / 'stage' / 'generic' / 'default' /
-                'builtin' / 'SanityFailureCheck')
+                'builtin' / 'HelloTest')
+
+    # Place a new file in the stagedir to test (2)
     (stagedir / 'foobar').touch()
+    (stagedir_runonly / 'foobar').touch()
+
+    # Remove a not-needed file to test (2) and (3)
+    (stagedir / 'Makefile').unlink()
+    (stagedir_runonly / 'Makefile').unlink()
+
+    # Use `--max-retries` to test (4)
     returncode, stdout, stderr = run_reframe(
         checkpath=['unittests/resources/checks/frontend_checks.py'],
         more_options=['-n', 'SanityFailureCheck',
                       '--dont-restage', '--max-retries=1']
     )
-    assert os.path.exists(stagedir / 'foobar')
-    assert not os.path.exists(f'{stagedir}_retry1')
+    assert os.path.exists(stagedir_runonly / 'foobar')
+    assert not os.path.exists(stagedir_runonly / 'Makefile')
+    assert not os.path.exists(f'{stagedir_runonly}_retry1')
 
     # And some standard assertions
     assert 'Traceback' not in stdout