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

Update CLI Integration Tests to use JVM Test Suites #3580

Merged
merged 10 commits into from May 7, 2024

Conversation

adam-enko
Copy link
Contributor

@adam-enko adam-enko commented Apr 23, 2024

Part of KT-64200

  • Migrate the CLI integration tests away from the custom integration test convention (which is not build-cache compatible) to use JVM Test Suites.
  • Remove shadowing, replace with passing in the JARs via system properties. (a custom CommandLineArgumentProvider is confusing and overly complicated, but it's required to satisfy Gradle input normalization, so that the build-cache can be re-used.)
  • Simplify processUtils.kt - the coroutines logic isn't necessary since the entrypoint just used runBlocking {}.
  • move jsonBuilder.kt test-util into src/main, since it's not a test
  • Rename CliIntegrationTest to CliTest (it wasn't an integration test, and the name was confusing compared to the actual CliIntegrationTest)
  • The JSON config tests a .json in the resources directory. I thought this was strange, so I changed it to use a new file created in the JUnit temporary directory.

- Migrate the CLI integration tests away from the custom integration test convention (which are not build-cache compatible) to use JVM Test Suites.
- Remove shadowing, replace with passing in the JARs via system properties. (a custom CommandLineArgumentProvider is confusing and overly complicated, but it's required to satisfy Gradle input normalization, so that the build-cache can be re-used.)
- Simplify `processUtils.kt` - the coroutines logic isn't necessary since the entrypoint just used `runBlocking {}`.
- move `jsonBuilder.kt` test-util into `src/main`, since it's not a test
- Rename `CliIntegrationTest` to `CliTest` (it wasn't an integration test, and the name was confusing compared to the actual `CliIntegrationTest`)
@adam-enko adam-enko marked this pull request as ready for review April 24, 2024 12:33
@adam-enko adam-enko added the infrastructure Everything related to builds tools, CI configurations and project tooling label Apr 24, 2024
Copy link
Contributor

@whyoleg whyoleg left a comment

Choose a reason for hiding this comment

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

Overall LGTM, some minor questions mostly

@adam-enko adam-enko merged commit bf056d6 into master May 7, 2024
12 checks passed
@adam-enko adam-enko deleted the KT-64200/cli-jvm-test-suite branch May 7, 2024 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Everything related to builds tools, CI configurations and project tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants