Require all tests to follow naming convention #12252

pull ajtowns wants to merge 1 commits into bitcoin:master from ajtowns:rename_tests_no_leeway changing 1 files +4 −10

ajtowns commented at 8:58 PM on January 23, 2018: member

Based on top of #11774
MarcoFalke commented at 9:33 PM on January 23, 2018: member

utACK 4de3d52e655902a07c85db84f0c6236302ef78a4
promag commented at 9:44 PM on January 23, 2018: member

utACK 4de3d52.
fanquake added the label Tests on Jan 23, 2018
ajtowns force-pushed on Jan 25, 2018
ajtowns commented at 12:14 AM on January 25, 2018: member

Rebased
MarcoFalke commented at 9:12 PM on January 25, 2018: member

Could you also fix the names in .travis.yml for the cron job: --exclude pruning,dbcrash, please?

in test/functional/test_runner.py:480 in 0b94082361 outdated

 488 | -    if len(bad_script_names) < EXPECTED_VIOLATION_COUNT:
 489 | -        print("{}HURRAY!{} Number of functional tests violating naming convention reduced!".format(BOLD[1], BOLD[0]))
 490 | -        print("Consider reducing EXPECTED_VIOLATION_COUNT from %d to %d" % (EXPECTED_VIOLATION_COUNT, len(bad_script_names)))
 491 | -    elif len(bad_script_names) > EXPECTED_VIOLATION_COUNT:
 492 | -        print("INFO: %d tests not meeting naming conventions (expected %d):" % (len(bad_script_names), EXPECTED_VIOLATION_COUNT))
 493 | +    if len(bad_script_names) > 0:

jnewbery commented at 6:43 PM on January 26, 2018:

more pythonic would be if bad_script_names: (https://www.python.org/dev/peps/pep-0008/#id51: "For sequences, (strings, lists, tuples), use the fact that empty sequences are false.")

in test/functional/test_runner.py:483 in 0b94082361 outdated

 492 | -        print("INFO: %d tests not meeting naming conventions (expected %d):" % (len(bad_script_names), EXPECTED_VIOLATION_COUNT))
 493 | +    if len(bad_script_names) > 0:
 494 | +        print("%sERROR:%s %d tests not meeting naming conventions:" % (BOLD[1], BOLD[0], len(bad_script_names)))
 495 |          print("  %s" % ("\n  ".join(sorted(bad_script_names))))
 496 | -        assert len(bad_script_names) <= EXPECTED_VIOLATION_COUNT + LEEWAY, "Too many tests not following naming convention! (%d found, expected: <= %d)" % (len(bad_script_names), EXPECTED_VIOLATION_COUNT)
 497 | +        assert len(bad_script_names) == 0, "Some tests are not following naming convention!"

jnewbery commented at 6:47 PM on January 26, 2018:

This condition has already been tested. Just raise an assertion error:

raise AssertionError("Some tests are not following naming convention!")

jnewbery commented at 6:51 PM on January 26, 2018: member

Tested ACK 0b94082361d4ffe130766aa14f15dc7144f8239c with a couple of style nits.

Agree with Marco that the travis file needs to be updated.

I think this may need to be rebased to show that only the last commit is new.
ajtowns commented at 12:48 PM on January 29, 2018: member

@MarcoFalke ah, sorry about that. #12292 should fix it.
MarcoFalke commented at 8:51 PM on January 29, 2018: member

Are you planning on fixing up the nits?
[tests] Require all tests to follow naming convention 125f4a4909
ajtowns force-pushed on Jan 30, 2018
ajtowns commented at 8:59 AM on January 30, 2018: member

Nits fixed as suggested, and rebased to make github happier
MarcoFalke commented at 1:00 PM on January 30, 2018: member

utACK 125f4a49097062dccc7da2825a4e0c891384f588
MarcoFalke merged this on Jan 30, 2018
MarcoFalke closed this on Jan 30, 2018
MarcoFalke referenced this in commit 8d57319863 on Jan 30, 2018
PastaPastaPasta referenced this in commit 4f77eff89c on Dec 16, 2020
PastaPastaPasta referenced this in commit aae3da8cd3 on Dec 18, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#11774 [tests] Rename functional tests