-
Notifications
You must be signed in to change notification settings - Fork 82
fix: fallback to default test data path if GAR_TEST_DATA not set #758
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi @bhavish00007 , please note that this PR is not ready yet |
|
@yangxk1 can u give me update when its ready |
|
@bhavish00007, the check ci were failed, please format code (run |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #758 +/- ##
============================================
+ Coverage 77.77% 82.00% +4.22%
- Complexity 234 299 +65
============================================
Files 25 25
Lines 918 1000 +82
Branches 49 84 +35
============================================
+ Hits 714 820 +106
+ Misses 167 125 -42
- Partials 37 55 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hey @bhavish00007 , I formatted the code and pass the test, It looks good. Do you have anything else to add? |
|
Does this PR resolve the issue? @Thespica |
|
Hi @shivendra-dev54 , I think it is not enough. From the issue point of view, the most basic implementation solution is to also process the test path in the |
|
@yangxk1 |
|
That's great @shivendra-dev54 |
Reason for this PR
Fixes #757
Currently, the test data path for Spark testing is read solely from the
GAR_TEST_DATAenvironment variable. This creates friction for new contributors or developers running tests locally without a custom setup.This PR proposes a fallback to automatically detect common default locations (
testing/,../testing/,../../testing/) in case the environment variable or system property is not set.What changes are included in this PR?
checkTestData()method to:GAR_TEST_DATAenvironment variablegar.test.datatesting/, etc.)ldbc_sample/csv/ldbc_sample.graph.ymlexists before accepting the pathAre these changes tested?
These changes reuse the existing validation logic already in place within
checkTestData(). The fallback will only apply ifGAR_TEST_DATAis not set, so no changes are required in the test suite itself.Manually tested by running without the environment variable and verifying that the fallback path works correctly.
Are there any user-facing changes?
No. This only impacts the developer testing experience. No changes to APIs or public behavior.