feat: Add HTTP Server TCK #8499

sdelamo · 2022-12-19T14:30:21Z

Adds a new module http-server-tck

This module contains multiple tests for a Micronaut HTTP Server. Those tests were ported from the AWS Module.

It has an API ServerUnderTest and ServerUnderTestProvider which allows registering a module, implementing the TCK easily via a service loader.

It contains a test module test-suite-http-server-tck-netty which runs the TCK against micronaut-http-server-netty.

Adds a new module `http-server-tck` This module contains multiple tests for a Micronaut HTTP Server. Those tests were ported from the AWS Module. It has an API `ServerUnderTest` and `ServerUnderTestProvider` which allows registering a module, implementing the TCK easily via a service loader. It contains a test module `test-suite-http-server-tck-netty` which runs the TCK against `micronaut-http-server-netty`.

yawkat

the whole test harness seems a bit convoluted. couldnt these be normal junit tests that access the server implementation using a rule or something?

Delete Test in this commit are moved to the TCK micronaut-projects/micronaut-core#8499

sdelamo · 2022-12-19T14:51:08Z

the whole test harness seems a bit convoluted. couldnt these be normal junit tests that access the server implementation using a rule or something?

Not sure what you are proposing. Please, create a PR with this one as the base with your proposal

See linked PRs, specially in azure and lambda to see ServerUnderTest implementations for non EmbeddedServer solutions.

yawkat · 2022-12-19T15:03:09Z

i dont mean the ServerUnderTest necessarily, i just want the tests to not be default methods in interfaces. e.g. by configuring junit to run the tests from the main tck module in the netty module.

@melix what do you think, is there a better way to achieve this?

dstepanov · 2022-12-19T15:09:08Z

Why can't we have ordinary Spock tests as we have before? It should have been enough to copy existing Netty tests, and different implementations would provide a server implementation.

dstepanov · 2022-12-19T15:10:10Z

I feel like you created some testing framework when it's not needed at all.

melix · 2022-12-19T15:10:54Z

It should be possible to have tests exposed as a transitive dependency. By default Gradle will only add the tests from src/test on test classpath, but we can setup things differently, so that a test suite is added. The thing is how you get the thing under test injected into those tests. We can probably have several options here (like a static setter, or a Java service loader).

yawkat · 2022-12-19T15:11:55Z

@melix getting the right server implementation injected is already solved using ServiceLoader in this PR. i think that approach is fine.

melix · 2022-12-19T15:14:12Z

Right so it should only be a matter of:

having a test suite exposed as a consumable dependency
having that test suite injected as a dependency on the test classpath of the projects under test

I think we already do something like this in micronaut-data.

dstepanov · 2022-12-19T15:17:23Z

There are a few ways:

Create abstract tests like Micronaut Data and extend them in the actual implementation, something like NettyFormSpec extends AbstractFormSpec
Share tests with a hack just like in TechEmpower project https://github.com/TechEmpower/FrameworkBenchmarks/blob/master/frameworks/Java/micronaut/buildSrc/src/main/groovy/io.micronaut.benchmark.module.gradle

yawkat · 2022-12-19T15:19:01Z

extending each test is not workable here, since we will have tck implementations in different repositories. it's too easy to miss a test.

sdelamo · 2022-12-19T15:29:10Z

The requisite is that we need to publish a library with tests. And use that library in different repositories (servlet, azure, azure).

I think it is safer to publish a standard java library with interfaces and consume it s proposed in this PR than complicating the build.

To run the TCK means just creating a class which implements an interface and a ServerUnderTestProvider implementation.

See https://github.com/micronaut-projects/micronaut-aws/pull/1538/files#diff-8f69b875a6148983a3f6bbbdb3d29a4883825e685afaeea7a35dd9f3b3d20c72

Moreover, if you are not compliant with a test you can Disable the test see above example.

Honestly, I think it is a good solution it makes trivial to implement it in other modules. See linked PRs.

About creating a testing framework.

The test is in this PR are based on the tests that were in the AWS Module. I had to rewrite them so that they are HTTP server framework agnostic. I wrote helper classes (TestScenario, HTTPResponseAssertion) to remove a lot of duplication and to handle things such as bean pollution. We had a lot of duplication and tests difficult to read and these classes allow you to write HTTP Server tests as:

TestScenario.builder()
.specName("baba")
.request(....)
.assertion()
.run()

But using TestScenario is optional. internally uses the same concepts we use through the framework. HTTP Request, exchanges, assertions of status, body, headers.

sdelamo · 2022-12-19T15:32:13Z

The AWS PR is a good example of consuming the library:

https://github.com/micronaut-projects/micronaut-aws/pull/1538/files#

Right now, it has http-server-tck duplicated as well. But once this gets published in core, I will remove http-server-tck in that PR.

yawkat · 2022-12-19T15:37:36Z

i think it is better to have to do small modifications to the build in dependent modules, if it means we can use normal testing patterns: classes instead of interfaces, automatic discovery instead of registering each test class, maybe even spock, and maybe even being able to click the green arrow in intellij to run a test...

i see that in the aws pr, you disabled an individual test, which is indeed more convenient with your approach. however you could also do this with a normal exclusion rule.

imo you should work with @melix to find a good approach to keep the build as clean as possible but still allow normal testing patterns. (the techempower approach seems hacky, i dont know)

sdelamo · 2022-12-19T15:42:30Z

and maybe even being able to click the green arrow in intellij to run a test...

If you want to run a single test in your tck. You create a class and class AWSHelloTest implement HelloWorldTest and click the green button. That works fine or even rerun only the failed tests in the TCK.

sdelamo · 2022-12-19T15:46:38Z

, maybe even spock,

I like Spock. However, I think it is easier to maintain and to publish a library based on JUnit than in Spock.

melix · 2022-12-19T15:48:46Z

I agree with @yawkat that we should publish a test suite. It shouldn't really be complicated to do it, and it can use Spock too. Consuming shouldn't be complicated either. Especially if the consumers are only Micronaut projects.

graemerocher · 2022-12-19T16:02:21Z

Would like to point out that there is one other advantage of using JUnit/Java in that we can run these TCK tests with native image.

sdelamo · 2022-12-20T00:58:08Z

if it means we can use normal testing patterns: classes instead of interfaces, automatic discovery instead of registering each test class, maybe even spock, and maybe even being able to click the green arrow in intellij to run a test...

I have changed the PR to use @Suite. Let me know if this addresses your concerns.

yawkat

thanks!

yawkat · 2022-12-20T09:36:27Z

http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/BodyArgumentTest.java

+import org.junit.jupiter.api.Test;
+import java.io.IOException;
+
+@SuppressWarnings({


maybe we can turn of sonar/checkstyle for these modules? we don't use them for spock tests either, they're just gonna be a pita...

http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/BodyTest.java

dstepanov · 2022-12-20T09:51:29Z

http-server-tck/src/main/java/io/micronaut/http/server/tck/AssertionUtils.java

+    "java:S5960", // We're allowed assertions, as these are used in tests only
+})
+@Experimental
+public final class AssertionUtils {


This module should be internal

No, other modules may want to use it.

Yes, but not outside of Micronaut org

dstepanov · 2022-12-20T09:52:12Z

http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/BodyArgumentTest.java

+import org.junit.jupiter.api.Test;
+import java.io.IOException;
+
+@SuppressWarnings({


http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/FilterErrorTest.java

http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/FiltersTest.java

dstepanov · 2022-12-20T09:56:08Z

http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/MiscTest.java

+    "checkstyle:MissingJavadocType",
+    "checkstyle:DesignForExtension"
+})
+public class MiscTest {


I would separate tests into packages named after it's test scenarios: .filters, .reactive, .form etc

I agree we could improve how the TCK is structured. We can do this after this his been merged. The tests were ported from AWS, where there was no clear structure.

http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/MiscTest.java

test-suite-http-server-tck-netty/src/test/resources/logback.xml

sonarcloud · 2022-12-20T16:19:19Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
28 Code Smells

92.7% Coverage
0.0% Duplication

sdelamo requested review from alvarosanchez, yawkat and dstepanov December 19, 2022 14:30

yawkat reviewed Dec 19, 2022

View reviewed changes

sdelamo added a commit to micronaut-projects/micronaut-aws that referenced this pull request Dec 19, 2022

test: use HTTP Server TCK for function-aws-api-proxy

b4df9b6

Delete Test in this commit are moved to the TCK micronaut-projects/micronaut-core#8499

sdelamo mentioned this pull request Dec 19, 2022

test: use HTTP Server TCK for function-aws-api-proxy micronaut-projects/micronaut-aws#1538

Merged

alvarosanchez removed their request for review December 19, 2022 16:21

sdelamo added this to the 3.8.0 milestone Dec 20, 2022

use @suite

9dd45de

sdelamo requested a review from yawkat December 20, 2022 00:58

sdelamo added 2 commits December 20, 2022 01:59

Update build.gradle

8be6232

allow assertions

7eff7d2

sdelamo added 2 commits December 20, 2022 07:22

annotate with @Experimiental

71632d9

define SPEC_NAME constant

ebf86e9

sdelamo requested a review from melix December 20, 2022 09:30

yawkat approved these changes Dec 20, 2022

View reviewed changes

dstepanov requested changes Dec 20, 2022

View reviewed changes

sdelamo added 5 commits December 20, 2022 11:04

log: remove logger

28462ad

use constant

ee76cb8

extract constants

4a1e672

use consant

68dc6e2

use static methods to reduce repetition

1f50932

sdelamo requested a review from dstepanov December 20, 2022 10:38

dstepanov approved these changes Dec 20, 2022

View reviewed changes

sdelamo added 2 commits December 20, 2022 13:31

fix chekcstyle and javadoc

1256cb7

sonar: use ifPresent and entrySet

861002d

sdelamo merged commit 9a87098 into 3.8.x Dec 20, 2022

sdelamo deleted the http-server-tck branch December 20, 2022 16:09

sdelamo mentioned this pull request Dec 20, 2022

Tests to check support for application/x-www-form-urlencoded MediaType micronaut-projects/micronaut-gcp#666

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add HTTP Server TCK #8499

feat: Add HTTP Server TCK #8499

sdelamo commented Dec 19, 2022 •

edited

yawkat left a comment

sdelamo commented Dec 19, 2022 •

edited

yawkat commented Dec 19, 2022

dstepanov commented Dec 19, 2022

dstepanov commented Dec 19, 2022

melix commented Dec 19, 2022

yawkat commented Dec 19, 2022

melix commented Dec 19, 2022

dstepanov commented Dec 19, 2022

yawkat commented Dec 19, 2022

sdelamo commented Dec 19, 2022 •

edited

sdelamo commented Dec 19, 2022

yawkat commented Dec 19, 2022

sdelamo commented Dec 19, 2022

sdelamo commented Dec 19, 2022

melix commented Dec 19, 2022

graemerocher commented Dec 19, 2022

sdelamo commented Dec 20, 2022

yawkat left a comment

yawkat Dec 20, 2022

dstepanov Dec 20, 2022

dstepanov Dec 20, 2022

sdelamo Dec 20, 2022

dstepanov Dec 20, 2022

dstepanov Dec 20, 2022

dstepanov Dec 20, 2022

sdelamo Dec 20, 2022

sonarcloud bot commented Dec 20, 2022

feat: Add HTTP Server TCK #8499

feat: Add HTTP Server TCK #8499

Conversation

sdelamo commented Dec 19, 2022 • edited

yawkat left a comment

Choose a reason for hiding this comment

sdelamo commented Dec 19, 2022 • edited

yawkat commented Dec 19, 2022

dstepanov commented Dec 19, 2022

dstepanov commented Dec 19, 2022

melix commented Dec 19, 2022

yawkat commented Dec 19, 2022

melix commented Dec 19, 2022

dstepanov commented Dec 19, 2022

yawkat commented Dec 19, 2022

sdelamo commented Dec 19, 2022 • edited

sdelamo commented Dec 19, 2022

yawkat commented Dec 19, 2022

sdelamo commented Dec 19, 2022

sdelamo commented Dec 19, 2022

melix commented Dec 19, 2022

graemerocher commented Dec 19, 2022

sdelamo commented Dec 20, 2022

yawkat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Dec 20, 2022

sdelamo commented Dec 19, 2022 •

edited

sdelamo commented Dec 19, 2022 •

edited

sdelamo commented Dec 19, 2022 •

edited