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

Allow defining @Nested test classes in a different class file #1750

Closed
vmassol opened this issue Jan 26, 2019 · 10 comments
Closed

Allow defining @Nested test classes in a different class file #1750

vmassol opened this issue Jan 26, 2019 · 10 comments

Comments

@vmassol
Copy link

vmassol commented Jan 26, 2019

Adding a feature request as suggested at:
https://gitter.im/junit-team/junit5?at=5c4c985d7b68f941022161c5

I like the Nesting feature of JUnit5 and the ability to have a single BeforeClass/AfterClass (I have some docker JUnit5 Extension that starts/stops a set of containers and do complex setup). However it leads to large classes. It would be great to be able to make the test class manageable (I currently over 1000 lines of test code which is too much for a single class).

Thus the ability to define the Nested class not as an inner class but as some external class would be awesome.

Thanks

@sormuras
Copy link
Member

sormuras commented Jan 27, 2019

Interesting idea.

How would such manageable test class look like?

class A {
  int state = 13;

  @Nested(C.class) class B {}

  @Nested C c = ... // ???
}

class C {
  final A a;
  C(A a) { this.a= a; }

  @Test c1() { assert a.state == 13; }
}

@sormuras
Copy link
Member

sormuras commented Jan 27, 2019

Work-around that works without adding a new feature to Jupiter: use Java's class inheritance feature.

class TopTests {

	int state = 13;

	@Test
	void top1() {}

	@Nested
	class A {
		@Test
		void a1() {}
	}

	@Nested
	class B extends C {

		B() {
			super(TopTests.this);
		}

		@Test
		void b1() {}
	}
}

class C {

	private final TopTests top;

	C(TopTests top) {
		this.top = top;
	}

	@Test
	void c1() {
		assert top.state == 13;
	}
}

Yields

image

...with all test methods declared in C appearing in the B container.

@sbrannen
Copy link
Member

Work-around that works without adding a new feature to Jupiter: use Java's class inheritance feature.

That was going to be my answer, but you beat me to it. 😉

Inheritance is the solution here, by extending (potentially abstract classes) and/or implementing interfaces that contain default methods.

In light of that, I am inclined to close this issue.

@junit-team/junit-lambda, thoughts?

@sormuras
Copy link
Member

In light of that, I am inclined to close this issue.

+1

@mmerdes
Copy link
Contributor

mmerdes commented Jan 27, 2019

Given the Solution above I would close it as well.

@vmassol
Copy link
Author

vmassol commented Jan 27, 2019

Thanks for the idea! I'll try your solution in the coming days and report how it goes/feels.

At first sight, it seems a bit hackish: Having to create a fake class just to extend another one and thus be able to reuse it feels not like some great design. I see it more like a workaround.

There's also the concept of Suite that needs to be factored in, and which is another way of being able to group test classes together but it's missing the hierarchy aspect. So IMO a good design solution would be to merge the 2 concepts (Suite + Nested). Maybe simply by allowing the @Nested annotation to be used in standard class definition and when a suite is used (with @SelectPackages() for example), the test hierarchy is constructed from all the test classes found and their defined @Nested annotations. Maybe that's already possible, haven't tried.

I'll do some experiments in the coming days and report here.

Thanks for the interesting discussion!

PS: Maybe what I'm saying is nonsense, I'm relatively new to JUnit5 :)

@sbrannen
Copy link
Member

Thanks for the idea! I'll try your solution in the coming days and report how it goes/feels.

OK. Thanks.

At first sight, it seems a bit hackish: Having to create a fake class just to extend another one and thus be able to reuse it feels not like some great design. I see it more like a workaround.

Yes, it is a bit hackish, but it works if your goal is to use @Nested classes as they were designed, namely to share state and setup from enclosing classes while simultaneously providing nested structure and reporting.

Please note that @Nested test classes were not designed with the idea that they would become huge. So if your nested test classes become unwieldy, you may want to consider using testing traits (i.e., interfaces with default methods) or implementing your own extension(s) to simplify matters.

There's also the concept of Suite that needs to be factored in, and which is another way of being able to group test classes together but it's missing the hierarchy aspect. So IMO a good design solution would be to merge the 2 concepts (Suite + Nested). Maybe simply by allowing the @Nested annotation to be used in standard class definition and when a suite is used (with @SelectPackages() for example), the test hierarchy is constructed from all the test classes found and their defined @Nested annotations. Maybe that's already possible, haven't tried.

No, that is not possible, and I do not foresee the team supporting @Nested on anything other than a non-static nested test class (i.e., an inner class).

Regarding suites, the team has that in the backlog. See all open issues with the suites label for details.

@sbrannen
Copy link
Member

closing this issue due to the above feedback.

@vmassol
Copy link
Author

vmassol commented Feb 14, 2019

I'll do some experiments in the coming days and report here.

@sormuras I've now tested it and it's working fine. FTR this is what we have: https://dev.xwiki.org/xwiki/bin/view/Community/Testing/DockerTesting/#HSuites

It's not perfect (having to extend a class is not very natural) but it's working and we're happy for now and will test suites when they're ready for prime time.

Thanks

@sormuras
Copy link
Member

Thanks for the feedback, Vincent. Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants