Improved tests

Author: overheadhunter

src/main/java/io/github/coffeelibs/tinyoauth2client/http/RedirectTarget.java

@@ -16,6 +16,7 @@
 import java.nio.channels.ServerSocketChannel;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
+import java.util.Objects;
 
 /**
  * A TCP connector to deal with the redirect response. We only listen for the expected response,
@@ -93,11 +94,11 @@ static void tryBind(ServerSocketChannel ch, int... ports) throws IOException {
 	}
 
 	public void setSuccessResponse(Response successResponse) {
-		this.successResponse = successResponse;
+		this.successResponse = Objects.requireNonNull(successResponse);
 	}
 
 	public void setErrorResponse(Response errorResponse) {
-		this.errorResponse = errorResponse;
+		this.errorResponse = Objects.requireNonNull(errorResponse);
 	}
 
 	public URI getRedirectUri() {

src/test/java/io/github/coffeelibs/tinyoauth2client/AuthFlowTest.java

@@ -60,7 +60,7 @@ public class WithMockedRedirectTarget {
 		@BeforeEach
 		@SuppressWarnings({"unchecked"})
 		public void setup() throws IOException {
-			authEndpoint = URI.create("https://login.example.com/?existing_param=existing-value");
+			authEndpoint = URI.create("https://login.example.com/");
 			redirectTarget = Mockito.mock(RedirectTarget.class);
 			redirectTargetClass = Mockito.mockStatic(RedirectTarget.class);
 			redirectTargetClass.when(() -> RedirectTarget.start(Mockito.any(), Mockito.any())).thenReturn(redirectTarget);
@@ -145,7 +145,26 @@ public void testWithErrorHtml() {
 		}
 
 		@Test
-		@DisplayName("authorize(...) succeeds if browser redirects to URI")
+		@DisplayName("authorize(...) with existing query string in authorization endpoint")
+		public void testAuthorizeWithExistingQueryParams() throws IOException {
+			authEndpoint = URI.create("https://login.example.com/?existing_param=existing-value");
+			var authFlow = AuthFlow.asClient("my-client");
+
+			var result = authFlow.authorize(authEndpoint, browser);
+
+			Assertions.assertInstanceOf(AuthFlow.AuthFlowWithCode.class, result);
+			var browsedUriCaptor = ArgumentCaptor.forClass(URI.class);
+			Mockito.verify(browser).accept(browsedUriCaptor.capture());
+			var browsedUri = browsedUriCaptor.getValue();
+			Assertions.assertNotNull(browsedUri);
+			Assertions.assertNotNull(browsedUri.getRawQuery());
+			var queryParams = URIUtil.parseQueryString(browsedUri.getRawQuery());
+			Assertions.assertEquals("existing-value", queryParams.get("existing_param"));
+			Assertions.assertEquals("code", queryParams.get("response_type"));
+		}
+
+		@Test
+		@DisplayName("authorize(...) with custom scopes")
 		public void testAuthorize() throws IOException {
 			var authFlow = AuthFlow.asClient("my-client");
 
@@ -158,7 +177,6 @@ public void testAuthorize() throws IOException {
 			Assertions.assertNotNull(browsedUri);
 			Assertions.assertNotNull(browsedUri.getRawQuery());
 			var queryParams = URIUtil.parseQueryString(browsedUri.getRawQuery());
-			Assertions.assertEquals("existing-value", queryParams.get("existing_param"));
 			Assertions.assertEquals(authFlow.clientId, queryParams.get("client_id"));
 			Assertions.assertEquals("code", queryParams.get("response_type"));
 			Assertions.assertEquals(redirectTarget.getCsrfToken(), queryParams.get("state"));

src/test/java/io/github/coffeelibs/tinyoauth2client/http/RedirectTargetTest.java

@@ -28,12 +28,50 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Executors;
 import java.util.concurrent.atomic.AtomicReference;
 
 @Timeout(value = 3)
+@SuppressWarnings("resource")
 public class RedirectTargetTest {
 
+	@Test
+	@DisplayName("start() doesn't accept a relative path")
+	public void testStartWithRelativePath() {
+		Assertions.assertThrows(IllegalArgumentException.class, () -> RedirectTarget.start("hello"));
+	}
+
+	@Test
+	@DisplayName("setSuccessResponse() fails on null")
+	public void testSetSuccessNull() throws IOException {
+		try (var target = RedirectTarget.start("/")) {
+			Assertions.assertThrows(NullPointerException.class, () -> target.setSuccessResponse(null));
+		}
+	}
+
+	@Test
+	@DisplayName("setSuccessResponse() succeeds on non-null")
+	public void testSetSuccessNonNull() throws IOException {
+		try (var target = RedirectTarget.start("/")) {
+			Assertions.assertDoesNotThrow(() -> target.setSuccessResponse(Response.empty(Response.Status.OK)));
+		}
+	}
+
+	@Test
+	@DisplayName("setErrorResponse() fails on null")
+	public void testSetErrorNull() throws IOException {
+		try (var target = RedirectTarget.start("/")) {
+			Assertions.assertThrows(NullPointerException.class, () -> target.setErrorResponse(null));
+		}
+	}
+
+	@Test
+	@DisplayName("setErrorResponse() succeeds on non-null")
+	public void testSetErrorNonNull() throws IOException {
+		try (var target = RedirectTarget.start("/")) {
+			Assertions.assertDoesNotThrow(() -> target.setErrorResponse(Response.empty(Response.Status.OK)));
+		}
+	}
+
 	@Test
 	@DisplayName("start() doesn't leak resource on error")
 	public void testStartExceptionally() throws IOException {
@@ -134,9 +172,11 @@ public void testRedirectError() throws IOException, InterruptedException, URISyn
 
 	@Test
 	@DisplayName("http response 400 for missing code")
-	public void testRedirectMissingCode() throws IOException, InterruptedException {
+	public void testRedirectMissingCode() throws IOException, InterruptedException, URISyntaxException {
 		try (var redirect = RedirectTarget.start("/")) {
-			var uri = redirect.getRedirectUri();
+			var baseUri = redirect.getRedirectUri();
+			var query = "state=" + redirect.getCsrfToken();
+			var uri = new URI(baseUri.getScheme(), baseUri.getAuthority(), baseUri.getPath(), query, baseUri.getFragment());
 
 			var accessToken = receiveAsync(redirect);
 			var request = HttpRequest.newBuilder(uri).GET().build();
@@ -211,20 +251,21 @@ public void testInterrupt() throws IOException, InterruptedException {
 			var threadStarted = new CountDownLatch(1);
 			var threadExited = new CountDownLatch(1);
 			var exception = new AtomicReference<Exception>();
-			var future = Executors.newSingleThreadExecutor().submit(() -> {
+			var thread = new Thread(() -> {
 				try {
 					threadStarted.countDown();
-					return redirect.receive();
+					redirect.receive();
 				} catch (IOException e) {
 					exception.set(e);
 					throw new UncheckedIOException(e);
 				} finally {
 					threadExited.countDown();
 				}
 			});
+			thread.start();
 
 			threadStarted.await();
-			future.cancel(true);
+			thread.interrupt();
 			threadExited.await();
 
 			Assertions.assertInstanceOf(ClosedByInterruptException.class, exception.get());