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

Create a JUnit 5 Internal module and post ShamrockUnitTest for our in… #571

Merged

Conversation

stuartwdouglas
Copy link
Member

…ternal testing purposes

This moves our internal Shrinkwrap based tests to JUnit 5

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Just a few things I noted with a quick review.

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.wildfly.common.Assert.assertTrue;
Copy link
Member

Choose a reason for hiding this comment

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

This import does not seem right.

@@ -114,6 +115,15 @@ public void handleRequest(HttpServerExchange exchange) throws Exception {
return new RuntimeValue<>(d);
}

public static SocketAddress getHttpAddress() {
for (Undertow.ListenerInfo info : undertow.getListenerInfo()) {
if (info.getProtcol().equals("http") && info.getSslContext() == null) {
Copy link
Member

Choose a reason for hiding this comment

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

Not related to this patch but there's a typo here: s/getProtcol()/getProtocol()/

There are now three different runners:

- junit5: our preferred runner for end users
- junit4: legacy runner for JUnit 4
- junit5-internal: unit test runner for testing Shamrock itself
@stuartwdouglas stuartwdouglas merged commit 387825c into quarkusio:master Jan 22, 2019
@cescoffier cescoffier added this to the 0.7.0 milestone Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants