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

Make testScalarFunctions directly executable in test runners. #36

Closed
pj-spoelders opened this issue Feb 14, 2024 · 2 comments · Fixed by #37
Closed

Make testScalarFunctions directly executable in test runners. #36

pj-spoelders opened this issue Feb 14, 2024 · 2 comments · Fixed by #37
Assignees
Labels
refactoring Code improvement without behavior change

Comments

@pj-spoelders
Copy link
Collaborator

pj-spoelders commented Feb 14, 2024

Currently you need to run the whole encompassing ScalarFunctionsTestBase derived test class to also run the nested WithAutomaticParameterDiscovery class and its very convoluted testScalarFunctions test from test runner UIs (tried intellij's and eclipse's).
The nested class and its method do not show up there. Probably because these are part of the abstract ScalarFunctionsTestBase class, which is an abstract class (as this test framework was designed).

This could be improved.

As a bonus: the current testScalarFunctions test uses a caching system which is overly complex. It should be seen if this can somehow be improved. Probably not a simple task as it depends on trying out various combinations and then caching those that DO work on the Exasol database to retry later to speed up things.

@pj-spoelders pj-spoelders added the refactoring Code improvement without behavior change label Feb 14, 2024
@pj-spoelders
Copy link
Collaborator Author

pj-spoelders commented Feb 14, 2024

The bigger problem here is this nested class has a different junit test lifecycle: @TestInstance(PER_CLASS)
This makes fixing this issue hard without breaking existing implementations.
When we remove the nested class as scope and change the encompassing junit test lifecycle the existing static beforeAll and afterAll defined in derived classes are not called. This is a breaking change. However I think it beats other alternatives which will require more intensive refactoring and also will result in breakage.

@pj-spoelders pj-spoelders self-assigned this Feb 15, 2024
@pj-spoelders pj-spoelders changed the title Make WithAutomaticParameterDiscovery test runnable by itself Make testScalarFunctions directly executable in test runners. Feb 15, 2024
@pj-spoelders
Copy link
Collaborator Author

Solved in the following way:

  • The nested class is now removed.
  • The setup and teardown of the required infrastructure in the derived classes now needs to be moved to the abstract beforeAllSetup and afterAllTeardown methods that need to be implemented.
  • This is a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code improvement without behavior change
Projects
None yet
1 participant