MAINT: refactoring existing e2e tests on trace data ingestion #512
Conversation
Signed-off-by: qchea <[email protected]>
Signed-off-by: qchea <[email protected]>
Signed-off-by: qchea <[email protected]>
Signed-off-by: qchea <[email protected]>
Signed-off-by: qchea <[email protected]>
Signed-off-by: qchea <[email protected]>
|
Note: I see some flaky CI failure with OpenSearchSink integTest which is not touched by this PR. |
e2e/build.gradle
Outdated
| } | ||
|
|
||
| ext { | ||
| data_prepper_jar_filepath = "build/bin/${DATA_PREPPER_CORE_JAR.archiveName}" |
There was a problem hiding this comment.
We use Groovy for Gradle build files and should follow the correct coding standards. This should be camelCase: dataPrepperJarFilepath.
Also, this value should be scoped correctly. I'm guessing this should be the root project. In which case it should look more like "${project.rootProject.buildDir}/bin/${DATA_PREPPER_CORE_JAR.archiveName}".
There was a problem hiding this comment.
Actually it should be the scope of qa-test project, I will replace it with project(':qa-test').buildDir
There was a problem hiding this comment.
Instead of that, please use ${project.buildDir} which will use the current project.
e2e/build.gradle
Outdated
| integrationTestRuntime.extendsFrom testRuntime | ||
| } | ||
|
|
||
| def DATA_PREPPER_CORE_JAR = project(':data-prepper-core').jar |
There was a problem hiding this comment.
I try to avoid defs in Gradle builds. Is there a way to clean this up?
There was a problem hiding this comment.
I think it is not bad practice since it appeared in docs: https://docs.gradle.org/current/userguide/more_about_tasks.html
There was a problem hiding this comment.
I don't think it is bad practice, but I do try to avoid them when possible. But, if there isn't a better way, that is fine too.
Also, this isn't a constant, so it should probably also be camelCase: dataPrepperCoreJar.
settings.gradle
Outdated
| @@ -38,4 +38,6 @@ include 'data-prepper-plugins:blocking-buffer' | |||
| include 'data-prepper-plugins:http-source' | |||
| include 'data-prepper-plugins:grok-prepper' | |||
| include 'data-prepper-logstash-configuration' | |||
| include 'e2e' | |||
There was a problem hiding this comment.
I want to raise the question of what name to use here. This uses e2e, but there may be some other options: e2e-test, qa, qa-test, test.
I don't like the name test since it is ambiguous with the other test tasks. I do think I like the idea of a clearer name like e2e-test or qa-test since e2e may not be an immediately obvious convention.
Also, the core OpenSearch project uses qa for its end-to-end tests.
There was a problem hiding this comment.
@dlvenable I also went back and forth between qa and e2e. I think qa-test looks good to me.
There was a problem hiding this comment.
@cmanning09 @graytaylor0 , Any thoughts on the name?
There was a problem hiding this comment.
Actually e2e-test might be better since e2e is already populated in the repo 😄
Signed-off-by: qchea <[email protected]>
Signed-off-by: qchea <[email protected]>
Signed-off-by: qchea <[email protected]>
|
Thanks for making this change. This is a very nice improvement to the project structure! |
…512) * REF: refactoring existing trace e2e tests Signed-off-by: qchea <[email protected]> * MAINT: github workflow Signed-off-by: qchea <[email protected]> * ADD: README Signed-off-by: qchea <[email protected]> * FIX: spotless Signed-off-by: qchea <[email protected]> * STY: spotless for markdown Signed-off-by: qchea <[email protected]> * MAINT: address PR comments Signed-off-by: qchea <[email protected]> * MAINT: directory name Signed-off-by: qchea <[email protected]> * MAINT: build directory reference Signed-off-by: qchea <[email protected]>
Description
This PR
(1) refactored the existing trace e2e tests out from data-prepper-core to the new e2e module
(2) updated github workflow on e2e
Issues Resolved
[List any issues this PR will resolve]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.