Introduce new module for shared interfaces #174
Conversation
epsstan
requested review from
pierredebelen,
aormerod-gs,
davidharte-gs,
akphi,
MauricioUyaguari,
kevin-m-knight-gs and
hardikmaheshwari
as code owners
|
@akphi - Can we please include this in the next release of legend-engine ? Thanks |
|
Initial use cases for this module : (1) Mark new/wip apis for authn (2) Rename/simplify test classes/surefire configurations for tests such as Test_Relational_DbSpecific_Athena_UsingPureClientTestSuite, ExternalIntegration_TestConnectionAcquisitionWithFlowProvider_Databricks |
|
|
||
| Used to indicate a test that can only be run on a CI/CD server because that test requires configuration (e.g. secrets) that are injected by the CI/CD server. | ||
| */ | ||
| public interface RunUsingCICDServer |
There was a problem hiding this comment.
Why we need this one? My worry is that extending this interface could lead executing wrong categories. Like, do we will ever enable RunUsingCICDServer or we will enable RunUsingGithubActions and/or RunUsingTeamCity?
There was a problem hiding this comment.
Not everyone uses Github or TeamCity. But we can always extract a base interface .
There was a problem hiding this comment.
But if this is for the platform development, we should target what we use. Are aiming for people to use these generically? Even if that is the case, I will avoid extending this interface on the other two categories.
|
|
||
| Used to indicate integration tests that test database functionality e.g. SQL generation, authentication etc | ||
| */ | ||
| public interface DatabaseIntegrationTest |
There was a problem hiding this comment.
What is the difference between this and IntegrationTest? Why we need both?
There was a problem hiding this comment.
One is more specific than the other ? For e.g for SQL tests, we want the surefire plugin to select a set of tests. I'm thinking of even more fine grained categories.
There was a problem hiding this comment.
Maybe we annotate the Junit categoies as @experimentalapi :-D
There was a problem hiding this comment.
What is the value for that? Like, where you think we will run Integration test cases, but not SQL or vice-versa?
We will have an increase likelihood of not running test cases if we make this a complicated set of toggles.
We should be looking at this from the "running any test" POV so that we can try to make as intelligent decisions as possible when scheduling. The closest you get to what is happening on the test, the harder will get to make further refactoring on how things are executed.
Introduce new module for shared interfaces