From c296d176e379bce458805215a0968f3bdd04b8aa Mon Sep 17 00:00:00 2001 From: Lukas Forer Date: Fri, 24 May 2024 12:50:40 +0200 Subject: [PATCH 1/2] Add flag do filter dependencies --- .gitignore | 1 + docs/tutorials/github-actions.md | 18 ++++- .../nf/test/commands/RunTestsCommand.java | 5 ++ .../lang/dependencies/DependencyGraph.java | 1 - .../lang/dependencies/DependencyResolver.java | 35 +++++--- .../nf/test/lang/dependencies/IMetaFile.java | 21 +++++ .../test/lang/dependencies/SnapshotFile.java | 5 ++ .../nf/test/lang/dependencies/TestFile.java | 48 +++++++++++ .../nf/test/nextflow/NextflowScript.java | 5 ++ .../dependencies/DependencyResolverTest.java | 79 ++++++++++++++++++- .../test/lang/dependencies/IMetaFileTest.java | 31 ++++++++ 11 files changed, 233 insertions(+), 16 deletions(-) create mode 100644 src/test/java/com/askimed/nf/test/lang/dependencies/IMetaFileTest.java diff --git a/.gitignore b/.gitignore index 08be52db..68d924e3 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ test_mock.nf site /tests /.nf-test.log +/temp \ No newline at end of file diff --git a/docs/tutorials/github-actions.md b/docs/tutorials/github-actions.md index 382044fb..64a30a65 100644 --- a/docs/tutorials/github-actions.md +++ b/docs/tutorials/github-actions.md @@ -48,7 +48,7 @@ jobs: sudo mv nf-test /usr/local/bin/ - name: Run Tests - run: nf-test test + run: nf-test test --ci ``` ### Explanation: @@ -57,7 +57,7 @@ jobs: 2. **Set up JDK 11**: Uses the `actions/setup-java@v2` action to set up Java Development Kit version 11. 3. **Setup Nextflow**: Uses the `nf-core/setup-nextflow@v1` action to install the latest-edge version of Nextflow. 4. **Install nf-test**: Downloads and installs nf-test. -5. **Run Tests**: Runs nf-test without sharding. +5. **Run Tests**: Runs nf-test with the `--ci` flag. This activates the CI mode. Instead of automatically storing a new snapshot as per usual, it will now fail the test if no reference snapshot is available. This enables tests to fail when a snapshot file was forgotten to be committed. ## Step 2: Extending to Use Sharding @@ -95,7 +95,7 @@ jobs: sudo mv nf-test /usr/local/bin/ - name: Run Tests (Shard ${{ matrix.shard }}/${{ strategy.job-total }}) - run: nf-test test --shard ${{ matrix.shard }}/${{ strategy.job-total }} + run: nf-test test --ci --shard ${{ matrix.shard }}/${{ strategy.job-total }} ``` ### Explanation of Sharding: @@ -141,7 +141,7 @@ jobs: sudo mv nf-test /usr/local/bin/ - name: Run Tests (Shard ${{ matrix.shard }}/${{ strategy.job-total }}) - run: nf-test test --shard ${{ matrix.shard }}/${{ strategy.job-total }} --changed-since HEAD^ + run: nf-test test --ci --shard ${{ matrix.shard }}/${{ strategy.job-total }} --changed-since HEAD^ ``` ### Explanation of Changes: @@ -172,6 +172,16 @@ config { This configuration ensures that critical changes always result in a comprehensive validation of the pipeline, providing additional confidence in your CI process. +## Step 5: Additional useful Options + +The `--filter` flag allows you to selectively run test cases based on their specified types. For example, you can filter tests by module, pipeline, workflow, or function. This is particularly useful when you have a large suite of tests and need to focus on specific areas of functionality. By separating multiple types with commas, you can run a customized subset of tests that match the exact criteria you're interested in, thereby saving time and resources. + +The `--related-tests` flag enables you to identify and execute all tests related to the provided `.nf` or `nf.test` files. This is ideal for scenarios where you have made changes to specific files and want to ensure that only the relevant tests are run. You can provide multiple files by separating them with spaces, which makes it easy to manage and test multiple changes at once, ensuring thorough validation of your updates. + +When the `--follow-dependencies` flag is set, the nf-test tool will automatically traverse and execute all tests for dependencies related to the files specified with the `--related-tests` flag. This ensures that any interdependent components are also tested, providing comprehensive coverage. This option is particularly useful for complex projects with multiple dependencies, as it bypasses the firewall calculation process and guarantees that all necessary tests are executed. + +The `--changed-until` flag allows you to run tests based on changes made up until a specified commit hash or branch name. By default, this parameter uses `HEAD`, but you can specify any commit or branch to target the changes made up to that point. This is particularly useful for validating changes over a specific range of commits, ensuring that all modifications within that period are tested comprehensively. + ## Summary 1. **Without Sharding**: A straightforward setup where all tests run in a single job. diff --git a/src/main/java/com/askimed/nf/test/commands/RunTestsCommand.java b/src/main/java/com/askimed/nf/test/commands/RunTestsCommand.java index 07fe6274..f3cfa6ee 100644 --- a/src/main/java/com/askimed/nf/test/commands/RunTestsCommand.java +++ b/src/main/java/com/askimed/nf/test/commands/RunTestsCommand.java @@ -13,6 +13,7 @@ import com.askimed.nf.test.lang.dependencies.Coverage; import com.askimed.nf.test.lang.dependencies.DependencyExporter; import com.askimed.nf.test.lang.dependencies.DependencyResolver; +import com.askimed.nf.test.lang.dependencies.IMetaFile; import com.askimed.nf.test.util.AnsiText; import com.askimed.nf.test.util.GitCommand; import org.slf4j.Logger; @@ -80,6 +81,9 @@ public class RunTestsCommand extends AbstractCommand { @Option(names = { "--follow-dependencies", "--followDependencies"}, description = "Follows all dependencies when related-tests is set.", required = false, showDefaultValue = Visibility.ALWAYS) private boolean followDependencies = false; + @Option(names = { "--filter" }, description = "Filter test cases by specified types (e.g., module, pipeline, workflow or function). Multiple types can be separated by commas.", required = false, showDefaultValue = Visibility.ALWAYS) + private String dependencies = "all"; + @Option(names = { "--only-changed", "--onlyChanged"}, description = "Runs tests only for those files which are modified in the current git repository", required = false, showDefaultValue = Visibility.ALWAYS) private boolean onlyChanged = false; @@ -188,6 +192,7 @@ public Integer execute() throws Exception { File baseDir = new File(new File("").getAbsolutePath()); DependencyResolver resolver = new DependencyResolver(baseDir); resolver.setFollowingDependencies(followDependencies); + resolver.setTargets(IMetaFile.TargetType.parse(dependencies)); if (onlyChanged || changedSince != null) { diff --git a/src/main/java/com/askimed/nf/test/lang/dependencies/DependencyGraph.java b/src/main/java/com/askimed/nf/test/lang/dependencies/DependencyGraph.java index 0c8ddfe6..9c8b0228 100644 --- a/src/main/java/com/askimed/nf/test/lang/dependencies/DependencyGraph.java +++ b/src/main/java/com/askimed/nf/test/lang/dependencies/DependencyGraph.java @@ -57,7 +57,6 @@ public void addFile(IMetaFile metaFile) { public void connectDependencies(){ for (Node node: nodes.values()) { for (String dependency: node.getMetaFile().getDependencies()) { - //addDependency(dependency, node.getFilename()); addDependency(dependency, node.getFilename()); } } diff --git a/src/main/java/com/askimed/nf/test/lang/dependencies/DependencyResolver.java b/src/main/java/com/askimed/nf/test/lang/dependencies/DependencyResolver.java index 81233ef7..28d21cc5 100644 --- a/src/main/java/com/askimed/nf/test/lang/dependencies/DependencyResolver.java +++ b/src/main/java/com/askimed/nf/test/lang/dependencies/DependencyResolver.java @@ -20,6 +20,8 @@ public class DependencyResolver { private boolean followingDependencies = false; + private Set targets = new HashSet(); + private static Logger log = LoggerFactory.getLogger(DependencyResolver.class); public DependencyResolver(File baseDir) { @@ -38,11 +40,15 @@ public void setFollowingDependencies(boolean followingDependencies) { this.followingDependencies = followingDependencies; } + public void setTargets(Set targets) { + this.targets = targets; + } + public List findAllTests() throws Exception { List results = new Vector(); for (IMetaFile metaFile: graph.getFiles()) { - if (metaFile.getType() == IMetaFile.MetaFileType.TEST_FILE) { + if (metaFile.getType() == IMetaFile.MetaFileType.TEST_FILE && acceptMetaFile(metaFile)) { results.add(new File(metaFile.getFilename())); } } @@ -61,7 +67,7 @@ public List findTestsByFiles(List files) throws Exception { List results = new Vector(); for (IMetaFile metaFile: graph.getFiles()) { - if (metaFile.getType() == IMetaFile.MetaFileType.TEST_FILE) { + if (metaFile.getType() == IMetaFile.MetaFileType.TEST_FILE && acceptMetaFile(metaFile)) { File file = new File(metaFile.getFilename()); TestFilePattern matchedPattern = matches(file.toPath(), patterns); if (matchedPattern != null) { @@ -102,7 +108,7 @@ public List findRelatedTestsByFiles(File ... files) throws Exception { long time0 = System.currentTimeMillis(); for (File file: files) { - results.addAll(findRelatedTestsByFile(file.getAbsoluteFile(), followingDependencies)); + results.addAll(findRelatedTestsByFile(file.getAbsoluteFile())); } long time1 = System.currentTimeMillis(); log.info("Found {} tests for file {} in {} sec", results.size(), files, (time1 - time0) / 1000.0); @@ -110,7 +116,7 @@ public List findRelatedTestsByFiles(File ... files) throws Exception { return new Vector(results); } - private Set findRelatedTestsByFile(File file, boolean followingDependencies) throws Exception { + private Set findRelatedTestsByFile(File file) throws Exception { Set results = new HashSet(); @@ -123,7 +129,9 @@ private Set findRelatedTestsByFile(File file, boolean followingDependencie // the file is a test file if (metaFile.getType() == IMetaFile.MetaFileType.TEST_FILE){ - results.add(new File(metaFile.getFilename())); + if (acceptMetaFile(metaFile)) { + results.add(new File(metaFile.getFilename())); + } return results; } @@ -134,21 +142,24 @@ private Set findRelatedTestsByFile(File file, boolean followingDependencie if (dependency.getType() == IMetaFile.MetaFileType.TEST_FILE) { // is a test file --> return - results.add(dependencyFile); + if (acceptMetaFile(dependency)) { + results.add(dependencyFile); + } } else { // if a source file DependencyGraph.Node node = graph.getNode(dependency.getFilename()); - //TODO: add && !followingDependencies if (node.hasDependencyOfType(IMetaFile.MetaFileType.TEST_FILE) && !followingDependencies) { //has a test --> add all test and then stop for (DependencyGraph.Node dependencyOfDependency: node.getDependencies()) { if (dependencyOfDependency.getMetaFile().getType() == IMetaFile.MetaFileType.TEST_FILE) { - results.add(new File(dependencyOfDependency.getFilename())); + if (acceptMetaFile(dependencyOfDependency.getMetaFile())) { + results.add(new File(dependencyOfDependency.getFilename())); + } } } } else { //has no tests --> find related tests in a recursive way - results.addAll(findRelatedTestsByFile(dependencyFile, followingDependencies)); + results.addAll(findRelatedTestsByFile(dependencyFile)); } } } @@ -266,5 +277,11 @@ public boolean matches2(Path path, List ignorePatterns) { return false; } + public boolean acceptMetaFile(IMetaFile file) { + if (targets == null || targets.isEmpty()) { + return true; + } + return targets.contains(file.getTarget()); + } } diff --git a/src/main/java/com/askimed/nf/test/lang/dependencies/IMetaFile.java b/src/main/java/com/askimed/nf/test/lang/dependencies/IMetaFile.java index b124a8ae..5cd2a6ed 100644 --- a/src/main/java/com/askimed/nf/test/lang/dependencies/IMetaFile.java +++ b/src/main/java/com/askimed/nf/test/lang/dependencies/IMetaFile.java @@ -1,6 +1,7 @@ package com.askimed.nf.test.lang.dependencies; import java.io.IOException; +import java.util.HashSet; import java.util.Set; public interface IMetaFile { @@ -9,6 +10,8 @@ public interface IMetaFile { public MetaFileType getType(); + public TargetType getTarget(); + public String getFilename(); public void parseDependencies() throws IOException; @@ -17,4 +20,22 @@ public static enum MetaFileType{ SOURCE_FILE, TEST_FILE, SNAPSHOT_FILE } + public static enum TargetType{ + PROCESS, WORKFLOW, PIPELINE, FUNCTION, UNDEFINED; + + public static Set parse(String targets) { + Set result = new HashSet(); + String cleaned = targets.trim().toUpperCase(); + if (cleaned.isEmpty() || cleaned.equalsIgnoreCase("ALL")) { + return result; + } + for (String target: cleaned.split(",")) { + String cleanedTarget = target.trim(); + result.add(TargetType.valueOf(cleanedTarget)); + } + return result; + } + + } + } diff --git a/src/main/java/com/askimed/nf/test/lang/dependencies/SnapshotFile.java b/src/main/java/com/askimed/nf/test/lang/dependencies/SnapshotFile.java index dd4251a8..537f0c91 100644 --- a/src/main/java/com/askimed/nf/test/lang/dependencies/SnapshotFile.java +++ b/src/main/java/com/askimed/nf/test/lang/dependencies/SnapshotFile.java @@ -45,4 +45,9 @@ public Set getDependencies() { return dependencies; } + @Override + public TargetType getTarget() { + return TargetType.UNDEFINED; + } + } diff --git a/src/main/java/com/askimed/nf/test/lang/dependencies/TestFile.java b/src/main/java/com/askimed/nf/test/lang/dependencies/TestFile.java index e8c5d26e..46495c81 100644 --- a/src/main/java/com/askimed/nf/test/lang/dependencies/TestFile.java +++ b/src/main/java/com/askimed/nf/test/lang/dependencies/TestFile.java @@ -1,13 +1,16 @@ package com.askimed.nf.test.lang.dependencies; import com.askimed.nf.test.util.FileUtil; +import org.apache.tools.ant.taskdefs.Tar; import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; import java.util.HashSet; +import java.util.List; import java.util.Set; +import java.util.Vector; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -19,6 +22,8 @@ public class TestFile implements IMetaFile { private Set dependencies = new HashSet(); + private TargetType target = TargetType.UNDEFINED; + public TestFile(File baseDir, File file) { this.baseDir = baseDir; this.file = file; @@ -26,6 +31,45 @@ public TestFile(File baseDir, File file) { public void parseDependencies() throws IOException { String script = FileUtil.readFileAsString(file); + target = parseType(script); + parseDependencies(script); + } + + private TargetType parseType(String script) { + + TargetType type = parseType(script, "nextflow_pipeline", TargetType.PIPELINE); + if (type != TargetType.UNDEFINED) { + return type; + } + type = parseType(script, "nextflow_process", TargetType.PROCESS); + if (type != TargetType.UNDEFINED) { + return type; + } + type = parseType(script, "nextflow_workflow", TargetType.WORKFLOW); + if (type != TargetType.UNDEFINED) { + return type; + } + type = parseType(script, "nextflow_function", TargetType.FUNCTION); + return type; + + } + + private TargetType parseType(String script, String type, TargetType targetType) { + + String patternType = "(?i)^\\s*" + type + "\\s*(.+)(\\s*\\{|\\{)"; + + Pattern r = Pattern.compile(patternType, Pattern.MULTILINE); + + Matcher m = r.matcher(script); + if (m.find()) { + return targetType; + } else { + return TargetType.UNDEFINED; + } + } + + private void parseDependencies(String script) throws IOException { + String regex = "(?i)script\\s+\"(.+)\""; Pattern pattern = Pattern.compile(regex); @@ -73,4 +117,8 @@ public Set getDependencies() { return dependencies; } + @Override + public TargetType getTarget() { + return target; + } } diff --git a/src/main/java/com/askimed/nf/test/nextflow/NextflowScript.java b/src/main/java/com/askimed/nf/test/nextflow/NextflowScript.java index 5289cc9e..ba70ea22 100644 --- a/src/main/java/com/askimed/nf/test/nextflow/NextflowScript.java +++ b/src/main/java/com/askimed/nf/test/nextflow/NextflowScript.java @@ -163,6 +163,11 @@ public static Set getDependencies(File file, String content) { } + @Override + public TargetType getTarget() { + return TargetType.UNDEFINED; + } + protected static Path resolve(File file, String dependency) { if (dependency.startsWith("./") || dependency.startsWith("../")) { return Paths.get(file.getParentFile().getAbsolutePath()).resolve(dependency); diff --git a/src/test/java/com/askimed/nf/test/lang/dependencies/DependencyResolverTest.java b/src/test/java/com/askimed/nf/test/lang/dependencies/DependencyResolverTest.java index 333d5bd1..0b152613 100644 --- a/src/test/java/com/askimed/nf/test/lang/dependencies/DependencyResolverTest.java +++ b/src/test/java/com/askimed/nf/test/lang/dependencies/DependencyResolverTest.java @@ -9,8 +9,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; -import java.util.List; -import java.util.Vector; +import java.util.*; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -83,6 +82,82 @@ void findRelatedTestAndFollowDependencies() throws Exception { assertEquals(17, coverage.getAll().getItems().size()); } + @Test + void findRelatedTestAndFilterDependencies() throws Exception { + + File root = getFetchNgs(); + + DependencyResolver resolver = new DependencyResolver(root); + resolver.setFollowingDependencies(false); + + resolver.buildGraph(); + assertEquals(10, resolver.findRelatedTestsByFiles( + new File(root, "modules/local/sra_to_samplesheet/main.nf") + ).size()); + + resolver = new DependencyResolver(root); + Set targets = new HashSet(); + targets.add(IMetaFile.TargetType.PROCESS); + resolver.setTargets(targets); + + resolver.buildGraph(); + assertEquals(1, resolver.findRelatedTestsByFiles( + new File(root, "modules/local/sra_to_samplesheet/main.nf") + ).size()); + + resolver = new DependencyResolver(root); + targets = new HashSet(); + targets.add(IMetaFile.TargetType.PROCESS); + targets.add(IMetaFile.TargetType.WORKFLOW); + resolver.setTargets(targets); + + resolver.buildGraph(); + assertEquals(10, resolver.findRelatedTestsByFiles( + new File(root, "modules/local/sra_to_samplesheet/main.nf") + ).size()); + + resolver = new DependencyResolver(root); + targets = new HashSet(); + targets.add(IMetaFile.TargetType.WORKFLOW); + resolver.setTargets(targets); + + resolver.buildGraph(); + assertEquals(9, resolver.findRelatedTestsByFiles( + new File(root, "modules/local/sra_to_samplesheet/main.nf") + ).size()); + + resolver = new DependencyResolver(root); + targets = new HashSet(); + targets.add(IMetaFile.TargetType.PIPELINE); + resolver.setTargets(targets); + resolver.setFollowingDependencies(true); + resolver.buildGraph(); + assertEquals(1, resolver.findRelatedTestsByFiles( + new File(root, "modules/local/sra_to_samplesheet/main.nf") + ).size()); + + resolver = new DependencyResolver(root); + targets = new HashSet(); + targets.add(IMetaFile.TargetType.PIPELINE); + targets.add(IMetaFile.TargetType.WORKFLOW); + resolver.setTargets(targets); + resolver.setFollowingDependencies(true); + resolver.buildGraph(); + assertEquals(10, resolver.findRelatedTestsByFiles( + new File(root, "modules/local/sra_to_samplesheet/main.nf") + ).size()); + + resolver = new DependencyResolver(root); + targets = new HashSet(); + targets.add(IMetaFile.TargetType.WORKFLOW); + resolver.setTargets(targets); + resolver.setFollowingDependencies(true); + resolver.buildGraph(); + assertEquals(9, resolver.findRelatedTestsByFiles( + new File(root, "modules/local/sra_to_samplesheet/main.nf") + ).size()); + } + @Test void findRelatedTestsWithTrigger() throws Exception { diff --git a/src/test/java/com/askimed/nf/test/lang/dependencies/IMetaFileTest.java b/src/test/java/com/askimed/nf/test/lang/dependencies/IMetaFileTest.java new file mode 100644 index 00000000..e52cdeac --- /dev/null +++ b/src/test/java/com/askimed/nf/test/lang/dependencies/IMetaFileTest.java @@ -0,0 +1,31 @@ +package com.askimed.nf.test.lang.dependencies; + +import org.junit.jupiter.api.Test; + +import java.util.HashSet; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.*; + +class IMetaFileTest { + + @Test + void parseDependencies() { + String input = "workflow,process"; + Set targets = new HashSet(); + targets.add(IMetaFile.TargetType.WORKFLOW); + targets.add(IMetaFile.TargetType.PROCESS); + assertEquals(targets , IMetaFile.TargetType.parse(input)); + } + + @Test + void parseDependenciesAll() { + String input = "all"; + Set targets = new HashSet(); + assertEquals(targets , IMetaFile.TargetType.parse(input)); + + input = " "; + targets = new HashSet(); + assertEquals(targets , IMetaFile.TargetType.parse(input)); + } +} \ No newline at end of file From cc190d4184e0513be4aab9952982c4194da90996 Mon Sep 17 00:00:00 2001 From: Lukas Forer Date: Fri, 24 May 2024 12:51:07 +0200 Subject: [PATCH 2/2] Update docs --- docs/docs/cli/test.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/docs/cli/test.md b/docs/docs/cli/test.md index 195c72fa..1ad30d2e 100644 --- a/docs/docs/cli/test.md +++ b/docs/docs/cli/test.md @@ -51,6 +51,11 @@ Writes test results in csv file. By default,nf-test automatically stores a new snapshot. When CI mode is activated, nf-test will fail the test instead of storing the snapshot automatically. +### `--filter ` + +Filter test cases by specified types (e.g., module, pipeline, workflow or function). Multiple types can be separated by commas. + + ### Optimizing Test Execution #### `--related-tests `