-
Notifications
You must be signed in to change notification settings - Fork 45
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
Feat(CI): Use environment variable to specify the location of testing data #512
Conversation
Hi, @lixueclaire @SemyonSinchenko , This PR may need your review, thanks. |
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
… data Signed-off-by: acezen <qiaozi.zwb@alibaba-inc.com> Fix the format and lint Signed-off-by: acezen <qiaozi.zwb@alibaba-inc.com>
d0440d3
to
79e7cff
Compare
|
||
```bash | ||
$ git clone https://github.com/apache/incubator-graphar-testing.git testing # download the testing data | ||
$ GAR_TEST_DATA=${PWD}/testing ctest |
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.
I would recommend to create a folder dev/
and provide bash/python scripts for downloading the data and set up all the variables. Like download_test_data.sh
, etc. We can do it in a separate PR. It is done like this in spark and I found it very user-friendly.
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.
The suggestion is great, I can do it in the PR #507 .
… data (apache#512) Signed-off-by: acezen <qiaozi.zwb@alibaba-inc.com>
Reason for this PR
as issue #510 describes, since the released source code tarball would not contains the submodules, to make the test work with tarball, it's better to deprecate testing data submodule and use developer setting environment variable to indicate the location of testing data.
What changes are included in this PR?
Are these changes tested?
already tested by current test case.
Are there any user-facing changes?
no