Details
- Reviewers
deadalnix - Group Reviewers
Restricted Project - Maniphest Tasks
- T163: test_runner.py should not list all the test, but instead be able to find them.
- Commits
- rSTAGING2d8bfe4d0dba: Search the filesystem to discover tests. Use the timing info from build and src…
rABC2d8bfe4d0dba: Search the filesystem to discover tests. Use the timing info from build and src…
Preconditions: make sure, that you do not build from src tree.
--> Test1: In an absence of timing.json, all tests should be run
--> Make sure that timing.json does not exists in any directory
find -name timing.json
build/test/functional/test_runner.py
cat build/timing.json
--> Check if all test are present in timing.json including both 'txn_doublespend.py --mineblock' and 'txn_doublespend.py'
--> Test2: Run with timings.json present:
--> This time timings.json will be used and only test shorter than 40 seconds will be run
build/test/functional/test_runner.py
cat build/timing.json
--> First few test that should be executed are wallet.py,abc-p2p-fullblocktest.py, wallet-hd.py,fundrawtransaction.py
--> dbcrash.py should not be executed
--> Test3: Let's run again, specifying --extended. All test should be executed, regardless of their timings
build/test/functional/test_runner.py --extended
cat build/timing.json
--> Test4: timings from src directory should be used if not found in build
mv build/timing.json test/functional/timing.json
build/test/functional/test_runner.py
--> long running test (dbcrash.py...) should not be executed. They should not be present in build/timing.json
cat build/timing.json
--> Test5: Timings from build should override timings from src
--> Edit build/timing.json, so that it only contains single line:
--> [{ "time": 0, "name": "dbcrash.py" }]
--> run without --extended flag, the test dbcrash should run,
--> because it is classified as not extended in build and extended in src
build/test/functional/test_runner.py
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- T163_small_commits2
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 2018 Build 2184: Bitcoin ABC Buildbot (legacy) Build 2183: arc lint + arc unit
Event Timeline
Overall, it looks good. A few small changes and it can get in.
| test/functional/test_runner.py | ||
|---|---|---|
| 195 ↗ | (On Diff #2982) | I wrote two comment that I deleted. This is very confusing. The timing object should definitively take a file rather than a folder. |
| 199 ↗ | (On Diff #2982) | I think this should be done by get_tests_to_run . I don't think there is a use case where we want one but not the other, so there is no point exposing both as functions. |
| test/functional/test_runner.py | ||
|---|---|---|
| 456 ↗ | (On Diff #3027) | You probably want to time test with each parameter differently. If that's hard to do, we should go ahead with the patch that way and make a task. If it is easy, you should do this. |
| test/functional/test_runner.py | ||
|---|---|---|
| 456 ↗ | (On Diff #3027) | The for loop starting in line 448 create multiple entries if a test is present in test_params. Example: it currently creates two entries for txn_doublespend and they are executed independently as they were two separate tests. The timing information is also recorded twice. {
"name": "txn_doublespend.py",
"time": 14
},
{
"name": "txn_doublespend.py --mineblock",
"time": 15
}Is this OK or did you expect something else? If you modify TEST_PARAMS so that it contains: "txn_doublespend.py": [["--mineblock"],["--anotherInvocation", "--withTwoParameters"] ], it will execute test three times and record three timing - for each of the following: It this OK? |
| test/functional/test_runner.py | ||
|---|---|---|
| 456 ↗ | (On Diff #3027) | yes this is to be the right behavior. |
Rebased on master to resolve merge conflicts (some tests were added to TEST_LIST, which is being deleted bz this DIFF)
I've also re-run all tests to get updated timing.json, which now includes the new tests.