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

Provide pioneer own AssertJ assertions #232

Closed
Bukama opened this issue Apr 21, 2020 · 5 comments · Fixed by #245
Closed

Provide pioneer own AssertJ assertions #232

Bukama opened this issue Apr 21, 2020 · 5 comments · Fixed by #245

Comments

@Bukama
Copy link
Member

Bukama commented Apr 21, 2020

When checking ExcecutionResults inside a test class for Throwables you have to go through the results and filter them. While doing this, already assertions are made that there are such results. Furthermore to avoid repeated codes some assertions were moved into a method inside the test class and were so hided when only looking at the acutal test case.

We also have some private assertThat... methods in some extension test classes which can be used in other tests so. Moreover there are no good assertThat(results) methods in the default AssertJ API. So in stream the idea came up to provide some own AssertJ assertion methods, to do something like the following.

//   TODO: assertThat(results).hasSingleTestFailedWith(ExtensionConfigurationException.class);
//   TODON'T: result.hasThrowable().isInstanceOf().hasMessage()?
     results.assertTestFailedWithExtensionConfigurationException();

Another one:

PioneerAnnotationUtilsTests:

		private void assertFailedTestHasMessage(ExecutionResults results, String... messages) {
			assertThat(results.numberOfFailedTests()).isEqualTo(1);
			assertThat(results.firstFailuresThrowableMessage()).isEqualTo(String.join(",", messages));
		}

Examples can be seen at this tutorial

@Michael1993
Copy link
Member

I've been thinking about writing an entire separate part for single test assertions (because we do a LOT of single test assertions)
So instead of saying:

assertThat(results).hasSingleStartedTest();
assertThat(results).hasNumberOfFailedTests(0);
assertThat(results).hasSingleSuccessfulTest();

We could write:

assertThat(results).hasSingleTest().thatStarted().thatSucceeded();

Or maybe even:

assertThat(results).hasSingleTest().thatStarted().thatSucceeded().butDidNot().fail();

But that seems slightly overkill to me.

@aepfli
Copy link
Member

aepfli commented Apr 29, 2020

Generally speaking, and from an API point of perspective. i would recommend to think what would a user need, and not just what is sufficient for our usecase. Hence that i would actually prefer a way where i can request a specific amount of tests like

assertThat(result).hasNumberOfTests(int amount)
assertThat(result).hasNumberOfTestsBetween(int start, int end)

and based on that we could further test for

....executed
....successful
....failed // or what ever naming

so we can use the same asserts on one or 2 or n test results. if we than create a wrapper method which is name .hasSingleTest() and is internally calling .hasNumberOfTests(1) is not the big magic after all. But in the future we could use this for more than just the single testcase, with imho little implementation effort. or do i miss a crucial part here?

@Michael1993
Copy link
Member

Michael1993 commented Apr 29, 2020

What I meant was:
Are there enough assertions that assert a single test to create a whole different interface for those assertions? You don't really answer that question in your answer (only tangentially).

Edit of shame: I misunderstood what you meant. Of course we could also have the same interface for multiple tests:

assertThat(results).hasNumberOfTests(2).thatStarted().thatFailed();

Did you mean it like that? If so, you are absolutely right.

@Michael1993
Copy link
Member

Where do we stop, though? You could even do something like this:

assertThat(results).hasSingleTest().that.wasStarted().and.hasFailed();

But that seems like a lot of effort for something that's meant to be used internally only.
I mean, I'd love to do it, but it does kind of look insane (and not really practical).

@aepfli
Copy link
Member

aepfli commented Apr 29, 2020

well the initial goal is to use it internally, but what if your code and the usage is so beautiful and intuitive that this might even be used in other junit5 extension for assertions :) who knows.

Bukama pushed a commit that referenced this issue Jul 14, 2020
In 0b79db8 (#6 / #218), we moved all assertion-like methods onto the
`ExecutionResults` class even though that wasn't a good fit. The
intention was to collect all such methods to then more easily replace
them with proper AssertJ-like assertions that we needed to write
ourselves.

This change implements these methods. The API is pretty good already,
but we expect that after using it for a while the experience with it
as well as new use cases may lead to further changes (see #298).

Closes: #232
PR: #245
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants