-
Notifications
You must be signed in to change notification settings - Fork 25
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
Added script to install multiple dependencies from submodules and then run E2E tests from a submodule #144
base: develop
Are you sure you want to change the base?
Conversation
d8bca97
to
826c862
Compare
826c862
to
4e66196
Compare
plugin_name = root.find('{http://maven.apache.org/POM/4.0.0}artifactId').text | ||
plugin_version = root.find('{http://maven.apache.org/POM/4.0.0}version').text | ||
|
||
os.chdir('target') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need to build the dependency module
first then only we can install it from the target
folder right??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the entire project needs to be built using mvn package
before installing all dependencies and running E2E tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this script isn't taking care of that.
The older script first builds the plugin module and then installs it.
ref - https://github.com/cdapio/cdap-e2e-tests/blob/develop/src/main/scripts/run_e2e_test.py#L69
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be the responsibility of the plugin? With multi-module projects there may be optimizations (such as only building certain modules) or using multiple threads when compiling. I wanted to leave this for the parent project to decide how to handle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed offline to keep it as the responsibility of caller that module is built before running the script.
Can we add a comment here in the script for future reference??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
43ca293
to
09efcdb
Compare
efad402
to
7f9578c
Compare
This can be useful in multi-module projects if structured like:
Note that the parent project and all submodules must be build before using this script.
Example usage in https://github.com/data-integrations/google-cloud/blob/gcp-modules-v3/.github/workflows/e2e.yml#L71