Merge branch 'feature/refactor-selected-test-script' into 'master'

feature/refactor-test-selection

See merge request !2626

(cherry picked from commit d2606b8f)

a4d5cad8 [bin] add script to detect modified files
3e12917e [bin][findtests] pass changed file list as arg
135007f9 [bin][findtests] use more appropriate flag
f12a5240 avoid obsolete dunecontrol call
a4659d04 [ci] add merge request branch info output
fe78beb1 [ci] pass more mr-related variables downstream
7770fbfa [ci] remove default value for target branch
4b2b969c [ci] print trigger source var in job
65f3548b [ci] decide build/test job based on selection job result
9ecda465 [bin][getchangedfiles] prefer subprocess' cwd over chdir
ae76d3d3 [bin][findtests] make functions accept build-dir
b972895b [bin][findtests] avoid caching mechanism

.gitlab-ci.yml

@@ -84,4 +84,6 @@ trigger lecture:
     branch: feature/test-dumux-trigger
     strategy: depend
   variables:
-    DUMUX_MERGE_REQUEST_BRANCH: $CI_MERGE_REQUEST_SOURCE_BRANCH_NAME
+    DUMUX_PIPELINE_SOURCE: $CI_PIPELINE_SOURCE
+    DUMUX_MERGE_REQUEST_SOURCE_BRANCH: $CI_MERGE_REQUEST_SOURCE_BRANCH_NAME
+    DUMUX_MERGE_REQUEST_TARGET_BRANCH: $CI_MERGE_REQUEST_TARGET_BRANCH_NAME

.gitlab-ci/default.yml

@@ -10,35 +10,38 @@ workflow:
   rules:
     - if: $CI_PIPELINE_SOURCE=="parent_pipeline"
 
+# variables that may be overwritten by the trigger
 variables:
-  TRIGGER_SOURCE: "unknown"
-  MR_TARGET_BRANCH_NAME: "master"
+  TRIGGER_SOURCE: "undefined"
+  MR_TARGET_BRANCH_NAME: "undefined"
 
 select tests:
   stage: configure
   script:
     - |
+      dunecontrol --opts=$DUNE_OPTS_FILE --current all
       if [[ "$TRIGGER_SOURCE" == "merge_request_event" ]]; then
-          dunecontrol --opts=$DUNE_OPTS_FILE --current all
-          pushd build-cmake
-            python3 ../bin/testing/findtests.py -f ../affectedtests.json -t origin/$MR_TARGET_BRANCH_NAME
-          popd
+          echo "Detecting changes w.r.t to target branch '$MR_TARGET_BRANCH_NAME'"
+          python3 bin/testing/getchangedfiles.py -o changedfiles.txt -t origin/$MR_TARGET_BRANCH_NAME
+          python3 bin/testing/findtests.py -o affectedtests.json --file-list changedfiles.txt --build-dir build-cmake
       else
+          echo "Received '$TRIGGER_SOURCE' as pipeline trigger event"
           echo "Skipping test selection, build/test stages will consider all tests!"
-          echo "{}" >> ../affectedtests.json
+          touch affectedtests.json
       fi
   artifacts:
     paths:
+      - build-cmake
       - affectedtests.json
     expire_in: 3 hours
 
 build dumux:
   stage: build
   script:
-    - dunecontrol --opts=$DUNE_OPTS_FILE --current all
     - |
       pushd build-cmake
-        if [[ "$TRIGGER_SOURCE" == "merge_request_event" ]]; then
+        make clean && make all
+        if [ -s ../affectedtests.json ]; then
           python3 ../bin/testing/runselectedtests.py -c ../affectedtests.json -b
         else
           python3 ../bin/testing/runselectedtests.py --all -b
@@ -61,7 +64,7 @@ test dumux:
   script:
     - |
       pushd build-cmake
-        if [[ "$TRIGGER_SOURCE" == "merge_request_event" ]]; then
+        if [ -s ../affectedtests.json ]; then
           python3 ../bin/testing/runselectedtests.py -c ../affectedtests.json -t
         else
           python3 ../bin/testing/runselectedtests.py --all -t

bin/testing/findtests.py

@@ -25,40 +25,36 @@ def hasCommonMember(myset, mylist):
 
 
 # make dry run and return the compilation command
-def getCompileCommand(testConfig):
-    lines = subprocess.check_output(["make", "--dry-run",
-                                     testConfig["target"]],
-                                    encoding='ascii').splitlines()
+def getCompileCommand(testConfig, buildTreeRoot='.'):
+    target = testConfig['target']
+    lines = subprocess.check_output(["make", "-B", "--dry-run", target],
+                                    encoding='ascii',
+                                    cwd=buildTreeRoot).splitlines()
 
     def hasCppCommand(line):
         return any(cpp in line for cpp in ['g++', 'clang++'])
 
+    # there may be library build commands first, last one is the actual target
     commands = list(filter(lambda line: hasCppCommand(line), lines))
-    assert len(commands) <= 1
-    return commands[0] if commands else None
+    return commands[-1] if commands else None
 
 
 # get the command and folder to compile the given test
-def buildCommandAndDir(testConfig, cache):
-    compCommand = getCompileCommand(testConfig)
+def buildCommandAndDir(testConfig, buildTreeRoot='.'):
+    compCommand = getCompileCommand(testConfig, buildTreeRoot)
     if compCommand is None:
-        with open(cache) as c:
-            data = json.load(c)
-            return data["command"], data["dir"]
+        raise Exception("Could not determine compile command for {}".format(testConfig))
     else:
         (_, dir), command = [comm.split() for comm in compCommand.split("&&")]
-        with open(cache, "w") as c:
-            json.dump({"command": command, "dir": dir}, c)
         return command, dir
 
 
 # check if a test is affected by changes in the given files
-def isAffectedTest(testConfigFile, changedFiles):
+def isAffectedTest(testConfigFile, changedFiles, buildTreeRoot='.'):
     with open(testConfigFile) as configFile:
         testConfig = json.load(configFile)
 
-    cacheFile = "TestTargets/" + testConfig["target"] + ".json"
-    command, dir = buildCommandAndDir(testConfig, cacheFile)
+    command, dir = buildCommandAndDir(testConfig, buildTreeRoot)
     mainFile = command[-1]
 
     # detect headers included in this test
@@ -68,19 +64,10 @@ def isAffectedTest(testConfigFile, changedFiles):
     headers = subprocess.run(command + ["-MM", "-H"],
                              stderr=PIPE, stdout=PIPE, cwd=dir,
                              encoding='ascii').stderr.splitlines()
+    headers = [h.lstrip('. ') for h in headers]
+    headers.append(mainFile)
 
-    # filter only headers from this project and turn them into relative paths
-    projectDir = os.path.abspath(os.getcwd().rstrip("build-cmake"))
-
-    def isProjectHeader(headerPath):
-        return projectDir in headerPath
-
-    testFiles = [os.path.relpath(mainFile.lstrip(". "), projectDir)]
-    testFiles.extend([os.path.relpath(header.lstrip(". "), projectDir)
-                      for header in filter(isProjectHeader, headers)])
-    testFiles = set(testFiles)
-
-    if hasCommonMember(changedFiles, testFiles):
+    if hasCommonMember(changedFiles, headers):
         return True, testConfig["name"], testConfig["target"]
 
     return False, testConfig["name"], testConfig["target"]
@@ -90,41 +77,37 @@ if __name__ == '__main__':
 
     # parse input arguments
     parser = ArgumentParser(description='Find tests affected by changes')
-    parser.add_argument('-s', '--source',
-                        required=False, default='HEAD',
-                        help='The source tree (default: `HEAD`)')
-    parser.add_argument('-t', '--target',
-                        required=False, default='master',
-                        help='The tree to compare against (default: `master`)')
+    parser.add_argument('-l', '--file-list', required=True,
+                        help='A file containing a list of files that changed')
     parser.add_argument('-np', '--num-processes',
                         required=False, type=int, default=4,
                         help='Number of processes (default: 4)')
-    parser.add_argument('-f', '--outfile',
+    parser.add_argument('-o', '--outfile',
                         required=False, default='affectedtests.json',
                         help='The file in which to write the affected tests')
+    parser.add_argument('-b', '--build-dir',
+                        required=False, default='.',
+                        help='The path to the top-level build directory of the project to be checked')
     args = vars(parser.parse_args())
 
-    # find the changes files
-    changedFiles = subprocess.check_output(
-        ["git", "diff-tree", "-r", "--name-only", args['source'],  args['target']],
-        encoding='ascii'
-    ).splitlines()
-    changedFiles = set(changedFiles)
+    buildDir = os.path.abspath(args['build_dir'])
+    targetFile = os.path.abspath(args['outfile'])
+    with open(args['file_list']) as files:
+        changedFiles = set([line.strip('\n') for line in files.readlines()])
 
     # clean build directory
-    subprocess.run(["make", "clean"])
-    subprocess.run(["make"])
-
-    # create cache folder
-    os.makedirs("TestTargets", exist_ok=True)
+    subprocess.run(["make", "clean"], cwd=buildDir)
+    subprocess.run(["make", "all"], cwd=buildDir)
 
     # detect affected tests
     print("Detecting affected tests:")
     affectedTests = {}
-    tests = glob("TestMetaData/*json")
+    tests = glob(os.path.join(buildDir, "TestMetaData") + "/*json")
 
     numProcesses = max(1, args['num_processes'])
-    findAffectedTest = partial(isAffectedTest, changedFiles=changedFiles)
+    findAffectedTest = partial(isAffectedTest,
+                               changedFiles=changedFiles,
+                               buildTreeRoot=buildDir)
     with Pool(processes=numProcesses) as p:
         for affected, name, target in p.imap_unordered(findAffectedTest, tests, chunksize=4):
             if affected:
@@ -133,5 +116,5 @@ if __name__ == '__main__':
 
     print("Detected {} affected tests".format(len(affectedTests)))
 
-    with open(args['outfile'], 'w') as jsonFile:
+    with open(targetFile, 'w') as jsonFile:
         json.dump(affectedTests, jsonFile)

bin/testing/getchangedfiles.py

+#!/usr/bin/env python3
+
+"""
+Get the names of the files that differ between two git trees
+"""
+
+import os
+import subprocess
+from argparse import ArgumentParser
+
+
+def getCommandOutput(command, cwd=None):
+    return subprocess.check_output(command, encoding='ascii', cwd=cwd)
+
+
+# get the files that differ between two trees in a git repo
+def getChangedFiles(gitFolder, sourceTree, targetTree):
+
+    gitFolder = os.path.abspath(gitFolder)
+    root = getCommandOutput(
+        command=['git', 'rev-parse', '--show-toplevel'],
+        cwd=gitFolder
+    ).strip('\n')
+    changedFiles = getCommandOutput(
+        command=["git", "diff-tree", "-r", "--name-only", sourceTree, targetTree],
+        cwd=gitFolder
+    ).splitlines()
+
+    return [os.path.join(root, file) for file in changedFiles]
+
+
+if __name__ == '__main__':
+
+    # parse input arguments
+    parser = ArgumentParser(
+        description='Get the files that differ between two git-trees'
+    )
+    parser.add_argument('-f', '--folder',
+                        required=False, default='.',
+                        help='The path to a folder within the git repository')
+    parser.add_argument('-s', '--source-tree',
+                        required=False, default='HEAD',
+                        help='The source tree (default: `HEAD`)')
+    parser.add_argument('-t', '--target-tree',
+                        required=False, default='master',
+                        help='The tree to compare against (default: `master`)')
+    parser.add_argument('-o', '--outfile',
+                        required=False, default='changedfiles.txt',
+                        help='The file in which to write the changed files')
+    args = vars(parser.parse_args())
+
+    changedFiles = getChangedFiles(args['folder'],
+                                   args['source_tree'],
+                                   args['target_tree'])
+
+    with open(args['outfile'], 'w') as outFile:
+        for file in changedFiles:
+            outFile.write(f"{os.path.abspath(file)}\n")