Skip to content

Conversation

@rawls238
Copy link
Contributor

@kmsquire

this cleans up the tests a bit so that most of the imports are done in runtests instead of duplicated across test files

@kmsquire
Copy link
Member

Actually, I would prefer to leave the imports at the top of each file. Without that, it's not possible to run individual tests from the command line--you'd have to run them all, and that's slightly annoying during development.

Also, it doesn't really hurt anything--if a file has already been imported, the second import is almost a no-op.

(But I really, really do appreciate trying to clean things up--thanks!)

@rawls238
Copy link
Contributor Author

@kmsquire one thing we could do to alleviate that is to do what the QuantEcon package does and optionally allow the tests to be run to be passed as command-line arguments. https://github.com/QuantEcon/QuantEcon.jl/blob/master/test/runtests.jl#L34

@kmsquire
Copy link
Member

Sure, that would be fine. The main Julia repo does something similar.

@DanielArndt
Copy link
Collaborator

I like the idea of being able to run tests in groups and individually (it can really cut down turnaround time if you're doing a little TDD or "test backed re-factoring" (TBR?), but some of the test files don't currently run independently.

@kmsquire
Copy link
Member

@DanielArndt, would you be okay with @rawls238 proposed solution of an extra command line parameter?

@DanielArndt
Copy link
Collaborator

Certainly. I think another feature that could possibly extend the model QuantEcon uses is to put individual tests in functions, and add the ability to run tests individually (instead of entire files).

We use a model similar to what I describe internally, and I've been meaning to look at where base Julia is headed for tests, and see if there was a benefit to extracting some of that out.

@rawls238 rawls238 force-pushed the guy_single_import_tests branch from 2785cc2 to 0d75294 Compare January 21, 2016 03:51
@rawls238
Copy link
Contributor Author

@DanielArndt that seems interesting to implement, but I think beyond the scope of this PR. Certainly something that would be nice to have. Let me know if you guys are good with merging this

@kmsquire
Copy link
Member

LGTM! Feel free to merge.

rawls238 pushed a commit that referenced this pull request Jan 21, 2016
@rawls238 rawls238 merged commit 8b01dfb into JuliaCollections:master Jan 21, 2016
@rawls238 rawls238 deleted the guy_single_import_tests branch January 21, 2016 04:26
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.

3 participants