Exclude src/test/ from shared libraries by default

[dwnusbaum]

#83 brought to my attention that some users are putting test code for their shared libraries in src/test, mostly when using https://github.com/jenkinsci/JenkinsPipelineUnit, which is not a good idea, since it means that the test code is available to Pipelines using the library. To help users who might have made this mistake accidentally (e.g. pipeline-library before jenkins-infra/pipeline-library#138), this PR makes it so that src/test is specifically excluded when a shared library is checked out.

In case users were actually using a a package named test in their shared library, there is a system property that can be set as an escape hatch to restore the previous behavior.

[jglick]

https://github.com/jenkinsci/archetypes/tree/master/global-shared-library/src/main/resources/archetype-resources is not affected AFAICT.

[daniel-beck]

Is there a way we can add a message to a build log when there's a test package that we're excluding? I.e. make this change discoverable?

[dwnusbaum]

@daniel-beck We could do something like this:

@@ -160,10 +163,22 @@ public class SCMSourceRetriever extends LibraryRetriever {
             // Cannot add WorkspaceActionImpl to private CpsFlowExecution.flowStartNodeActions; do we care?
             // Copy sources with relevant files from the checkout:
             String excludes = INCLUDE_SRC_TEST_IN_LIBRARIES ? null : "src/test/";
+            if (lease.path.act(new CheckForFilesInSrcTest())) {
+                listener.getLogger().println("Excluding src/test/ from checkout of shared library " + scm.getKey() + " so that test code cannot be accessed by Pipelines.");
+                listener.getLogger().println("To fix this issue, move the test code outside of src/. To disable this behavior, pass -D" + SCMSourceRetriever.class.getName() + ".INCLUDE_SRC_TEST_IN_LIBRARIES=true to the java command used to start Jenkins.");
+            }
             lease.path.copyRecursiveTo("src/**/*.groovy,vars/*.groovy,vars/*.txt,resources/", excludes, target);
         }
     }
 
+    private static class CheckForFilesInSrcTest extends SlaveToMasterFileCallable<Boolean> {
+        private static final long serialVersionUID = 1L;
+        @Override
+        public Boolean invoke(File dir, VirtualChannel channel) throws IOException, InterruptedException {
+            return new File(new File(dir, "src"), "test").exists();
+        }
+    }
+

Which would result in a message like this for users in their build logs:

Excluding src/test/ from checkout of git /Users/dnusbaum/Projects/workflow-cps-global-lib-plugin/target/tmp/junit1791104325590123496/junit9172288952035481749 so that test code cannot be accessed by Pipelines.
To fix this issue, move the test code outside of src/. To include these files in the checkout anyway, pass -Dorg.jenkinsci.plugins.workflow.libs.SCMSourceRetriever.INCLUDE_SRC_TEST_IN_LIBRARIES=true to the java command used to start Jenkins.

What do you think?

[daniel-beck]

@dwnusbaum It's unclear what issue would be fixed by moving files. How about this:

Excluding src/test/ from checkout of git /Users/dnusbaum/Projects/workflow-cps-global-lib-plugin/target/tmp/junit1791104325590123496/junit9172288952035481749 so that test code cannot be accessed by Pipelines.
To remove this log message, move the test code outside of src/. To restore the previous behavior that allowed access to files in src/test/, pass -Dorg.jenkinsci.plugins.workflow.libs.SCMSourceRetriever.INCLUDE_SRC_TEST_IN_LIBRARIES=true to the java command used to start Jenkins.

[v1v]

/LGTM