Parameters method should be invoked after BeforeClass/ClassRules #527

Closed
acfoltzer opened this Issue Oct 15, 2012 · 17 comments

Comments

Projects
None yet
8 participants
@acfoltzer

My use case: I want to open an expensive resource, and then use the data within to generate my parameters.

Currently, there is no way except in the body of a @Parameters-annotated method to specify setup that occurs before the parameters are returned. It seems more in keeping with the design of JUnit to handle this in @BeforeClass and @ClassRules code, so these should probably run before @Parameters methods.

@dsaff

This comment has been minimized.

Show comment
Hide comment
@dsaff

dsaff Oct 24, 2012

Member

I agree that there should be something better. However, the answer will probably be to leave @Parameters exactly as-is, and introduce a new, more Rule-like API. Stay tuned.

Member

dsaff commented Oct 24, 2012

I agree that there should be something better. However, the answer will probably be to leave @Parameters exactly as-is, and introduce a new, more Rule-like API. Stay tuned.

@AlexBischof

This comment has been minimized.

Show comment
Hide comment
@AlexBischof

AlexBischof Apr 21, 2015

Any update on this issue?

Any update on this issue?

@stefanbirkner

This comment has been minimized.

Show comment
Hide comment
@stefanbirkner

stefanbirkner Apr 21, 2015

Contributor

After reading the problem again, I don't see the need for extending the Parameterized runner.

The expensive resource can be easily opened in the @Parameters-annotated method. Using a special annotation wouldn't even save a single line of code. Instead it requires more knowledge about the annotation whereas the current approach is an plain call of a class method.

Currently:

private static void doSomethingBefore()  {
  ...
}

@Parameters
public static List<Object[]> data() {
  doSomethingBefore();
  ...
  return data;
}

With the new feature:

@NewAnnotation
private static void doSomethingBefore()  {
  ...
}

@Parameters
public static List<Object[]> data() {
  ...
  return data;
}

@AlexBischof How do you think about this?

Contributor

stefanbirkner commented Apr 21, 2015

After reading the problem again, I don't see the need for extending the Parameterized runner.

The expensive resource can be easily opened in the @Parameters-annotated method. Using a special annotation wouldn't even save a single line of code. Instead it requires more knowledge about the annotation whereas the current approach is an plain call of a class method.

Currently:

private static void doSomethingBefore()  {
  ...
}

@Parameters
public static List<Object[]> data() {
  doSomethingBefore();
  ...
  return data;
}

With the new feature:

@NewAnnotation
private static void doSomethingBefore()  {
  ...
}

@Parameters
public static List<Object[]> data() {
  ...
  return data;
}

@AlexBischof How do you think about this?

@Tibor17

This comment has been minimized.

Show comment
Hide comment
@Tibor17

Tibor17 Apr 21, 2015

Contributor

I don't think @NewAnnotation is the way the JUnit goes. Many frameworks use a strategy to configure the annotation with descriptive method.
A new descriptive method within @Parameters(priority = -1) does not require new annotation.
Similar to Java EE 7 @Priority(value = 400) ordering the interceptors wrapping the reflected method call or class.
How is multiple @Parameters handled by JUnit?

Contributor

Tibor17 commented Apr 21, 2015

I don't think @NewAnnotation is the way the JUnit goes. Many frameworks use a strategy to configure the annotation with descriptive method.
A new descriptive method within @Parameters(priority = -1) does not require new annotation.
Similar to Java EE 7 @Priority(value = 400) ordering the interceptors wrapping the reflected method call or class.
How is multiple @Parameters handled by JUnit?

@stefanbirkner

This comment has been minimized.

Show comment
Hide comment
@stefanbirkner

stefanbirkner Apr 21, 2015

Contributor

It only considers the first public static method that is annotated with @Parameters.

Contributor

stefanbirkner commented Apr 21, 2015

It only considers the first public static method that is annotated with @Parameters.

@stefanbirkner

This comment has been minimized.

Show comment
Hide comment
@stefanbirkner

stefanbirkner Apr 21, 2015

Contributor

I still don't understand how another annotated method is useful.

Contributor

stefanbirkner commented Apr 21, 2015

I still don't understand how another annotated method is useful.

@acfoltzer

This comment has been minimized.

Show comment
Hide comment
@acfoltzer

acfoltzer Apr 21, 2015

After this long, I'm struggling to remember exactly the context in which I made this request. To the best of my recollection I wanted to use an expensive resource in both the @Parameters-annotated method and some other method, and wanted some way to guarantee ordering between the two. Sorry I can't remember more; in any case this isn't an issue for me any longer.

After this long, I'm struggling to remember exactly the context in which I made this request. To the best of my recollection I wanted to use an expensive resource in both the @Parameters-annotated method and some other method, and wanted some way to guarantee ordering between the two. Sorry I can't remember more; in any case this isn't an issue for me any longer.

@AlexBischof

This comment has been minimized.

Show comment
Hide comment
@AlexBischof

AlexBischof Apr 22, 2015

Yesterday i just had a look on all parameterized labeled issues to get a feeling what is going on and this one felt a little bit lost.

For this issue i don't think that a new annotation is needed. The only thing i would like to point out is that classrules already are invoked before parameters but beforeclass not. This seems a little bit inconsistent ;)

Yesterday i just had a look on all parameterized labeled issues to get a feeling what is going on and this one felt a little bit lost.

For this issue i don't think that a new annotation is needed. The only thing i would like to point out is that classrules already are invoked before parameters but beforeclass not. This seems a little bit inconsistent ;)

@stefanbirkner

This comment has been minimized.

Show comment
Hide comment
@stefanbirkner

stefanbirkner Apr 22, 2015

Contributor

Thanks for bringing this issue back.

Your assumption about the invocation of @Parameters is wrong. This can be easily proven by running the test

@RunWith(Parameterized.class)
public class ParameterizedTest {
    @Parameters
    public static Object[] data() {
        System.out.println("@Parameters");
        return new Object[] { "one", "two"};
    }

    @Parameter(0)
    public String name;

    @ClassRule
    public static TestRule classRule = (base, description) -> {
        System.out.println("@ClassRule");
        return base;
    };

    @BeforeClass
    public static void beforeClass() {
        System.out.println("@BeforeClass");
    }

    @Test
    public void test() {
        System.out.println("test");
    }
}

Its output is

@Parameters
@ClassRule
@BeforeClass
test
test
Contributor

stefanbirkner commented Apr 22, 2015

Thanks for bringing this issue back.

Your assumption about the invocation of @Parameters is wrong. This can be easily proven by running the test

@RunWith(Parameterized.class)
public class ParameterizedTest {
    @Parameters
    public static Object[] data() {
        System.out.println("@Parameters");
        return new Object[] { "one", "two"};
    }

    @Parameter(0)
    public String name;

    @ClassRule
    public static TestRule classRule = (base, description) -> {
        System.out.println("@ClassRule");
        return base;
    };

    @BeforeClass
    public static void beforeClass() {
        System.out.println("@BeforeClass");
    }

    @Test
    public void test() {
        System.out.println("test");
    }
}

Its output is

@Parameters
@ClassRule
@BeforeClass
test
test
@AlexBischof

This comment has been minimized.

Show comment
Hide comment
@AlexBischof

AlexBischof Apr 23, 2015

Hm, you are right.
Strange...i had a (almost) similar test which gave me a different output. But i cannot find or reproduce it anymore.

Hm, you are right.
Strange...i had a (almost) similar test which gave me a different output. But i cannot find or reproduce it anymore.

@stefanbirkner

This comment has been minimized.

Show comment
Hide comment
@stefanbirkner

stefanbirkner Apr 23, 2015

Contributor

I close this issue. It is no longer a problem for @acfoltzer, who created the issue. IMHO it doesn't provide enough additional value that justifies extending the JUnit API. If you disagree, please reopen the issue.

Contributor

stefanbirkner commented Apr 23, 2015

I close this issue. It is no longer a problem for @acfoltzer, who created the issue. IMHO it doesn't provide enough additional value that justifies extending the JUnit API. If you disagree, please reopen the issue.

@AshwinJay

This comment has been minimized.

Show comment
Hide comment
@AshwinJay

AshwinJay Mar 10, 2017

I wanted to create some test data that is common to all my tests in a class. I wanted to parameterize my tests too. So, I thought I'd create it in the BeforeClass annotated method but alas! The BeforeClass method needed a TemporaryFolder annotated with ClassRule. So, I cannot create static data for my test class before of the unintuitive order in which the initialization happens.

@Parameters
@ClassRule
@BeforeClass
test
test

This would've been very useful:

@ClassRule
@BeforeClass
@Parameters
test
test

Edit: It looks like there is another case for fixing this: #671

AshwinJay commented Mar 10, 2017

I wanted to create some test data that is common to all my tests in a class. I wanted to parameterize my tests too. So, I thought I'd create it in the BeforeClass annotated method but alas! The BeforeClass method needed a TemporaryFolder annotated with ClassRule. So, I cannot create static data for my test class before of the unintuitive order in which the initialization happens.

@Parameters
@ClassRule
@BeforeClass
test
test

This would've been very useful:

@ClassRule
@BeforeClass
@Parameters
test
test

Edit: It looks like there is another case for fixing this: #671

@kcooney

This comment has been minimized.

Show comment
Hide comment
@kcooney

kcooney Mar 10, 2017

Member

@AshwinJay unfortunately, if we changed the ordering that these things happened, we would likely break someone. I suggest:

@ClassRule
public static final MyClassRule myClassRule = new MyClassRule();

@Parameters
public static Parameter[] data() {
    return new Parameter[] { ... };
}

public MyTest(Parameter parameter) {
  this.data = myClassRule.createData(parameter);
}
Member

kcooney commented Mar 10, 2017

@AshwinJay unfortunately, if we changed the ordering that these things happened, we would likely break someone. I suggest:

@ClassRule
public static final MyClassRule myClassRule = new MyClassRule();

@Parameters
public static Parameter[] data() {
    return new Parameter[] { ... };
}

public MyTest(Parameter parameter) {
  this.data = myClassRule.createData(parameter);
}
@AshwinJay

This comment has been minimized.

Show comment
Hide comment
@AshwinJay

AshwinJay Mar 10, 2017

Thanks but I wanted to createData before all the tests began, so this would not work.

Workaround:
I created a TemporaryFolder without the annotation and called the before() method myself as suggested here: #671 (comment)

Thanks but I wanted to createData before all the tests began, so this would not work.

Workaround:
I created a TemporaryFolder without the annotation and called the before() method myself as suggested here: #671 (comment)

@kcooney

This comment has been minimized.

Show comment
Hide comment
@kcooney

kcooney Mar 11, 2017

Member

@AshwinJay could you describe how your parameter data relates to the setup done in your @ClassRule?

Member

kcooney commented Mar 11, 2017

@AshwinJay could you describe how your parameter data relates to the setup done in your @ClassRule?

@AshwinJay

This comment has been minimized.

Show comment
Hide comment
@AshwinJay

AshwinJay Mar 14, 2017

@kcooney I wanted to create some test data in a directory before all the tests started.

  1. That meant using a public static TemporaryFolder annotated with @ClassRule
  2. I wanted to then create all the data in a method annotated with @BeforeClass and store the file names + some info in a private static TestData[]
  3. Finally I wanted the @Parameters annotated method to simply return the test data

@kcooney I wanted to create some test data in a directory before all the tests started.

  1. That meant using a public static TemporaryFolder annotated with @ClassRule
  2. I wanted to then create all the data in a method annotated with @BeforeClass and store the file names + some info in a private static TestData[]
  3. Finally I wanted the @Parameters annotated method to simply return the test data
@kcooney

This comment has been minimized.

Show comment
Hide comment
@kcooney

kcooney Mar 14, 2017

Member

@AshwinJay Parameterized is designed to have the parameters method generate the data. This is required so JUnit knows the number of test methods and so the test names can include the data in the Description. There are ways you can bend Parameterized to meet your use case, but they won't be simple.

Some ideas:

  • If the data you are writing is n files each with generated data, have the @Parameters method return the filenames (as in the name attribute of the File), and have the test method read the data in a @Before method.
  • If the filenames are not deterministic then the @Parameters method can return a sequence of integers and the test can list the directory and pick the nth file.
Member

kcooney commented Mar 14, 2017

@AshwinJay Parameterized is designed to have the parameters method generate the data. This is required so JUnit knows the number of test methods and so the test names can include the data in the Description. There are ways you can bend Parameterized to meet your use case, but they won't be simple.

Some ideas:

  • If the data you are writing is n files each with generated data, have the @Parameters method return the filenames (as in the name attribute of the File), and have the test method read the data in a @Before method.
  • If the filenames are not deterministic then the @Parameters method can return a sequence of integers and the test can list the directory and pick the nth file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment