sympy / sympy-bot-old

SymPy pull request helper
http://reviews.sympy.org/
Other
24 stars 16 forks source link

Add import time test #164

Open bgee opened 11 years ago

bgee commented 11 years ago

Add import speed test using bin/test_import.py.

asmeurer commented 11 years ago

You should parse the output of test_import and use the result from there (with the +-).

bgee commented 11 years ago

@asmeurer Just did. https://github.com/sympy/sympy/pull/1924 Also, test_import just assumes it will be called inside sympy/. Should I fix this in sympy? No big deal though. (Sorry I accidentally closed this pull request. )

asmeurer commented 11 years ago

Yes, fix it. Look at all the other various functions to see how (e.g., bin/test).

asmeurer commented 11 years ago

Could you add a flag to make this configurable (both command line and config file)?

bgee commented 11 years ago

@asmeurer Yes. I will try to finish this before tomorrow afternoon. What will be a good flag? We already have -t. Should we make it default to run? Also, I saw the test you put has a line Test command: ** -V**. It's not showing up on my machine. Did I break anything?

asmeurer commented 11 years ago

No, I tested with -t " -V" so that it wouldn't actually run the tests, so that it would come up quickly.

asmeurer commented 11 years ago

I guess call it --import-time or --no-import-time (depending on what you make the default).

bgee commented 11 years ago

@asmeurer I use --no-import-time for command line option and no_import_time = false/true in config file.

asmeurer commented 11 years ago

This looks fine. I want to just wait a little bit (maybe a few days) to see if anyone else has any comments.

asmeurer commented 11 years ago

By the way, you can test comments without spamming a pull request by using --no-comment (-n is also a good idea, since there's no point in uploading the reviews either), then pasting the comment output that is printed to the terminal in an issue in some test GitHub repository (I use https://github.com/asmeurer/GitHub-Issues-Test/issues).

bgee commented 11 years ago

@asmeurer Sounds good. I will also start working on that bin/test_import directory fix.