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

enable java 17 for testing project #903

Merged

Conversation

minesh-s-patel
Copy link
Contributor

@minesh-s-patel minesh-s-patel commented Jan 22, 2025

  • Moved integration tests using J17

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems unnecessary?

@@ -31,10 +31,10 @@ Workflow {
}
runtimeTest = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should actually refer to the test utility project. It's where Xtext will generate it's test utilities.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the MWE2 runtimeTest points to rosetta-testing
The issue is that it generates 2 things

  1. A Unit test (in rosetta-testing/src/test) - which is a Hello World test intended to be overriden.
  2. A type called RosettaInjectorProvider.java which (in rosetta-testing/src/main) which we need in the testing utils

When I just moved the all the tests to integration testing - it started to generate point 1 which caused a unit test failure.

When I move the MWE2 runtimeTest to point to rosetta-integration-tests, then we are generating point 2 int he wrong place.

I made a change to just disable the runtime test automation. The automation is meant to be a starting point and does not really (I think) provide much value any more.

The other option is that we move the MWE2 runtimeTest back to rosetta-testing, and check in the generated xtend test and live with 1 test being in rosetta-testing. I prefer the first option. @SimonCockx - take a look at the PR now and tell me what you think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good!

@minesh-s-patel minesh-s-patel merged commit f81de67 into finos:main Jan 22, 2025
5 checks passed
@minesh-s-patel minesh-s-patel deleted the enable-java-17-for-testing-project branch January 22, 2025 16:34
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.

2 participants