RFR: 2218: Automate execution of integration tests

[Zhao Song]

On Wed, 3 Apr 2024 09:04:28 GMT, Erik Duveblad <ehelin at openjdk.org> wrote:

> Hi all,
> 
> please review this patch that automates the execution of integration tests _if_ the required properties for the integration test are configured. Prior to this patch one would have to comment out the `@Disabled` notation and then make sure that all properties for all test methods where configured. It was also hard to see all properties required for a given test function to run. With this patch all properties are declared in an annotation and test functions whose properties are configured will be run automatically (no need to comment out any annotation).
> 
> I have also renamed these tests to end with the suffix `IntegrationTests.java`. For unit tests we use the suffix `Tests.java` and for tests that require properties to be configured we should use the suffix `IntegrationTests.javaa` (since that is what those tests are).
> 
> I also choose a different name for the file containing the properties required for a test to run, the file is now named `test.properties` instead of `manual-test-settings.properties`. The new name is quite a bit shorter and the tests are also no longer manual, they are run automatically as long the required properties for a given test are present.
> 
> I also added support for including test properties from another file to allow test properties to be shared among multiple developers. Included files must be specified with an absolute path since it is hard to figure out the current working directory that tests are being run from (it also make the configuration more explicit).
> 
> Lastly I have slightly refactored the integration tests so that they all follow a similar pattern. There quite a bit more work that needs to be done here: test properties use different naming schemes (camel case vs dot), not all test properties are prefix with the test suite name (could result in name clashes) and some of the test code could use some tender, love and care. This can be taken care of the later, this patch is already quite big.
> 
> Thanks,
> Erik
> 
> ### Testing
> - [x] Verified that tests are *not* run when properties are missing
> - [x] Verified that tests *are* run when properties are present
> - [x] Verified that tests are *not* run when no `test.properties` file is present
> - [x] Verified that all JIRA integration tests still pass

Looks pretty good and I really like it. Previously, running these tests was inconvenient. Now, this patch has made testing much simpler! Thanks for implementing it!

forge/src/test/java/org/openjdk/skara/forge/github/GitHubIntegrationTests.java line 26:

> 24: 
> 25: import java.time.Duration;
> 26: import java.util.Properties;

No longer need "import java.util.Properties"

forge/src/test/java/org/openjdk/skara/forge/gitlab/GitLabIntegrationTests.java line 28:

> 26: import java.time.ZonedDateTime;
> 27: import java.util.Arrays;
> 28: import java.util.Properties;

No longer need "import java.util.Properties"

forge/src/test/java/org/openjdk/skara/forge/gitlab/GitLabIntegrationTests.java line 331:

> 329:     @Test
> 330:     @EnabledIfTestProperties({"gitlab.user", "gitlab.pat", "gitlab.uri", "gitlab.group",
> 331:                               "gitlab.user"})

"gitlab.user" is duplicated

-------------

Marked as reviewed by zsong (Reviewer).

PR Review: https://git.openjdk.org/skara/pull/1627#pullrequestreview-1977505130
PR Review Comment: https://git.openjdk.org/skara/pull/1627#discussion_r1550169168
PR Review Comment: https://git.openjdk.org/skara/pull/1627#discussion_r1550168540
PR Review Comment: https://git.openjdk.org/skara/pull/1627#discussion_r1550161119