Skip to content

Test needs to assume script splitting is enabled #417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bitwiseman merged 2 commits into from
Jul 28, 2021
Merged

Test needs to assume script splitting is enabled #417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
051b530
Test needs to assume script splitting is enabled
bitwiseman Mar 11, 2021
a87693e
Merge branch 'master' into task/pct-failure
bitwiseman Jul 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/RuntimeASTTransformer.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ import static org.objectweb.asm.Opcodes.ACC_PUBLIC
* {@link ExecutionModelAction} to the run.
*/
@SuppressFBWarnings(value="SE_NO_SERIALVERSIONID")
class RuntimeASTTransformer {
public class RuntimeASTTransformer {
Copy link
Member

@jtnord jtnord Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guessing you do not actually want this to be public (except for test code?) so you can change it at will?

Suggested change
public class RuntimeASTTransformer {
@Restricted(NoExternalUse.class) // for test use only
public class RuntimeASTTransformer {

Copy link
Contributor Author

@bitwiseman bitwiseman Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jtnord
Actually, no. I considered that, but Groovy that is loaded and run in pipeline has difficulty with attributes. Making this public is not great, but not horrible.


/**
* Enables or disables the script splitting behavior in {@Wrapper} which
Expand Down
7 changes: 7 additions & 0 deletions ...c/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ASTParserUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,26 @@
import org.codehaus.groovy.control.SourceUnit;
import org.jenkinsci.plugins.pipeline.modeldefinition.BaseParserLoaderTest;
import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTPipelineDef;
import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer;
import org.junit.Assume;
import org.junit.Test;

import java.net.URL;
import java.security.CodeSource;
import java.security.cert.Certificate;

import static org.codehaus.groovy.control.Phases.CONVERSION;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;


public class ASTParserUtilsTest extends BaseParserLoaderTest {
@Test
public void prettyPrintTransformed() throws Exception {
// this test only works if script splitting is enabled
Assume.assumeThat(RuntimeASTTransformer.SCRIPT_SPLITTING_TRANSFORMATION, is(true));

URL src = getClass().getResource("/when/whenEnv.groovy");

// We need to do our own parsing logic here to make sure we do the runtime transformation, and so that we can
Expand Down