[SPARK-5482][PySpark] Allow individual test suites in python/run-tests #4269
Conversation
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
|
Another useful refactoring would be to let us run the tests for only one of the supported Python versions instead of all of them. This is useful when augmenting the test suite to collect coverage metrics. To do this, maybe we should split the script into two, one that tests with a particular python version and another that loops over the versions and invokes that script. |
|
ok, I understand it. I try to split run-tests and rebase this branch onto master. |
|
@JoshRosen Should we define the list of supported python versions in a test script?
|
|
@potix2, yep, that's the right set of versions to test against. In the top-level test runner script, I think that we can check whether those versions are installed (using By the way, if it's easier then I'd be fine with rewriting this file in Python rather than keeping it as bash. |
|
Thank you, @JoshRosen. I got it. There is no problem to rewrite in Python. |
potix2
changed the title
|
Jenkins, this is ok to test. |
|
Merged build triggered. |
|
Merged build started. |
|
Test build #35164 has started for PR 4269 at commit |
|
Hey @potix2, thanks for updating this. I'm going to take a look at this today because I have an outstanding PR to improve In the long run, it might be nice to rewrite this in Python, but I don't feel that such as rewrite necessarily has to be a blocker to merging this. If you're already working on a Python port, though, then just let me know and I'll be happy to wait for it. |
python/tests/pypy.sh
Outdated
There was a problem hiding this comment.
It looks like this pypy.sh file isn't invoked from anywhere; do we need it?
There was a problem hiding this comment.
Also, we might want to remove the # -*- coding: utf-8 -*- lines in case Apache RAT complains about them.
|
Test build #35164 has finished for PR 4269 at commit
|
|
Merged build finished. Test FAILed. |
|
Jenkins, retest this please. |
|
Merged build triggered. |
|
Merged build started. |
|
Test build #35179 has started for PR 4269 at commit |
|
Test build #35179 has finished for PR 4269 at commit
|
|
Merged build finished. Test PASSed. |
|
@JoshRosen, I haven't started rewriting in python yet, so it's no problem that I make it to be out of scope for this PR. |
|
Merged build triggered. |
|
Merged build started. |
|
Test build #35202 has started for PR 4269 at commit |
|
Test build #35202 has finished for PR 4269 at commit
|
|
Merged build finished. Test FAILed. |
potix2
changed the title
|
Merged build triggered. |
|
Merged build started. |
|
Test build #35206 has started for PR 4269 at commit |
|
Merged build triggered. |
|
Merged build started. |
|
Test build #35208 has started for PR 4269 at commit |
|
Test build #35206 has finished for PR 4269 at commit
|
|
Merged build finished. Test FAILed. |
|
Test build #35208 has finished for PR 4269 at commit
|
|
Merged build finished. Test PASSed. |
There was a problem hiding this comment.
Do we need to double-quote $1?
There was a problem hiding this comment.
By the way, this prints an error when I run it on my Mac:
[joshrosen python (2c6a7d4...)]$ ./run-tests
./run-tests: line 33: [: too many arguments
|
Actually, I find this bash code too difficult to understand / review and would rather just block on a complete Python rewrite. I don't want to spend the time to understand the bash-isms here if we're just going to rewrite anyways. |
|
Sorry to confuse you, I agree with you. As a first step, we should rewrite run-tests in Python, then append new features. I took a look at #6866, I think it has some useful functions to rewrite bash code into Python. If you don't mind, I want to wait to merge it. |
|
My other PR (#6866) was merged, so I think this should be unblocked now. Which functions were you hoping to borrow from that script? If we're going to use some of those functions in two places, then I guess we can either duplicate them or move them to a common location and import them from there. If we do choose to extract them into their own module, I'd probably opt to create a new Python module inside of the |
|
Hi @JoshRosen, no problem. Thank you for considering this PR. I try to revise my code. |
|
I ended up incorporating this feature into my pull request because I needed something like this in order to trigger a subset of the Python tests in the pull request builder. Therefore, I recommend that we close this PR and continue discussion on mine. Thanks! |
|
All right. Thanks! |
4 participants
Add options to run individual test suites in python/run-tests.
TODO
rewrite in python