Skip to content
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

Use os.path.splitext to check file extensions. #2681

Merged
merged 1 commit into from Aug 7, 2014
Merged

Use os.path.splitext to check file extensions. #2681

merged 1 commit into from Aug 7, 2014

Conversation

asakatida
Copy link
Contributor

File extensions were found by string parsing before. The search for tests and groups was also inconsistent. I have imported the standard library method for finding extensions. Both searches now assume a single directory contains all groups and tests.

@AtnNn
Copy link
Member

AtnNn commented Jul 11, 2014

Thanks! I didn't know about listdir or splitext. The changes look good to me, but I would like to check it out and test it before merging.

@asakatida
Copy link
Contributor Author

Testing is good when used. I did wonder if os.walk was in there because someone want to build a tree under test/full_test. Should I put both functions back to something more like load_groups?

@AtnNn
Copy link
Member

AtnNn commented Jul 11, 2014

@grandquista that may have been my idea originally. But I don't believe full_test will ever have more than one level deep. On the contrary, @larkost and I have talked about getting rid of it and using another method to specify the list of tests to run.

@mlucy
Copy link
Member

mlucy commented Jul 30, 2014

@AtnNn: what's the state of this pull request?

@mlucy mlucy added this to the tests milestone Jul 30, 2014
@AtnNn
Copy link
Member

AtnNn commented Aug 7, 2014

This patch seems to work great. Thanks!

AtnNn added a commit that referenced this pull request Aug 7, 2014
…n_test_name

Use `os.path.splitext` to check file extensions.
@AtnNn AtnNn merged commit 23749fb into rethinkdb:next Aug 7, 2014
@asakatida asakatida deleted the grandquista/Permit_period_in_test_name branch August 7, 2014 21:04
@AtnNn AtnNn modified the milestones: tests, 1.14 Aug 8, 2014
@larkost
Copy link
Collaborator

larkost commented Aug 27, 2014

@grandquista This has shipped with 1.14.0, thanks for the patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants