Skip to content
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.

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

[JENKINS-38306] Add a parameter to LocalData to use instead of method name #36

Merged

Conversation

2 participants
@ikedam
Copy link
Member

commented Sep 17, 2016

JENKINS-38306

It's often the case that I want to use a same local data in multiple test cases,
but not in all test cases in the test.
(e.g. some tests want to use other local data)

This change allows to specify the name of the directory to use
instead of method name

public class Test {
    @Rule public JenkinsRule j = new JenkinsRule();

    @Test
    @LocalData("config1")
    public void config1_test1() throws Exception {...}

    @Test
    @LocalData("config1")
    public void config1_test2() throws Exception {...}

    @Test
    @LocalData("config2")
    public void config2_test1() throws Exception {...}

    @Test
    @LocalData("config2")
    public void config2_test2() throws Exception {...}
}
@jglick

jglick approved these changes Oct 5, 2016

Copy link
Member

left a comment

The Java identifier checks look strange. Otherwise seems useful.

@@ -121,11 +126,31 @@ public File allocate() throws Exception {
return new CopyExisting(home).allocate();
}

public static boolean isJavaIdentifier(@CheckForNull String name) {

This comment has been minimized.

Copy link
@jglick

jglick Oct 5, 2016

Member

Can be replaced with SourceVersion.latestSupported().isIdentifier(name).

private URL findDataResource() {
// first, check method specific resource
Class<?> clazz = testMethod.getDeclaringClass();
String methodName = testMethod.getName();

if (isJavaIdentifier(alterName)) {

This comment has been minimized.

Copy link
@jglick

jglick Oct 5, 2016

Member

Why do we care whether it is a Java identifier or not? Should not matter for purposes of resource loading. Anyway, if it is unusable for some reason, we should fail with an error.

@jglick jglick merged commit 57a3162 into jenkinsci:master Oct 5, 2016

1 check passed

Jenkins This pull request looks good
Details
@ikedam

This comment has been minimized.

Copy link
Member Author

commented Oct 5, 2016

The Java identifier checks ensure that alterName doesn't contain special paths such as "..". I worry that those special paths might result unexpected behaviors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.