Skip to content

Conversation

ajtowns
Copy link
Contributor

@ajtowns ajtowns commented Jan 23, 2018

Based on top of #11774

@maflcko
Copy link
Member

maflcko commented Jan 23, 2018

utACK 4de3d52e655902a07c85db84f0c6236302ef78a4

@promag
Copy link
Contributor

promag commented Jan 23, 2018

utACK 4de3d52.

@fanquake fanquake added the Tests label Jan 23, 2018
@ajtowns ajtowns force-pushed the rename_tests_no_leeway branch from 4de3d52 to 0b94082 Compare January 25, 2018 00:13
@ajtowns
Copy link
Contributor Author

ajtowns commented Jan 25, 2018

Rebased

@maflcko
Copy link
Member

maflcko commented Jan 25, 2018

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

Copy link
Contributor

@jnewbery jnewbery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

print("Consider reducing EXPECTED_VIOLATION_COUNT from %d to %d" % (EXPECTED_VIOLATION_COUNT, len(bad_script_names)))
elif len(bad_script_names) > EXPECTED_VIOLATION_COUNT:
print("INFO: %d tests not meeting naming conventions (expected %d):" % (len(bad_script_names), EXPECTED_VIOLATION_COUNT))
if len(bad_script_names) > 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.")

print(" %s" % ("\n ".join(sorted(bad_script_names))))
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)
assert len(bad_script_names) == 0, "Some tests are not following naming convention!"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

@ajtowns
Copy link
Contributor Author

ajtowns commented Jan 29, 2018

@MarcoFalke ah, sorry about that. #12292 should fix it.

@maflcko
Copy link
Member

maflcko commented Jan 29, 2018

Are you planning on fixing up the nits?

@ajtowns ajtowns force-pushed the rename_tests_no_leeway branch from 0b94082 to 125f4a4 Compare January 30, 2018 08:56
@ajtowns
Copy link
Contributor Author

ajtowns commented Jan 30, 2018

Nits fixed as suggested, and rebased to make github happier

@maflcko
Copy link
Member

maflcko commented Jan 30, 2018

utACK 125f4a4

@maflcko maflcko merged commit 125f4a4 into bitcoin:master Jan 30, 2018
maflcko pushed a commit that referenced this pull request Jan 30, 2018
125f4a4 [tests] Require all tests to follow naming convention (Anthony Towns)

Pull request description:

  Based on top of #11774

Tree-SHA512: 1eb156b5a97b30c203b7b0ad3d2055b391ef825e2e57805c7745b5259a9b1caaa115768ec225452f12f354e550f83e071f9c6fee2c36698b4679191895aab8de
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 16, 2020
125f4a4 [tests] Require all tests to follow naming convention (Anthony Towns)

Pull request description:

  Based on top of bitcoin#11774

Tree-SHA512: 1eb156b5a97b30c203b7b0ad3d2055b391ef825e2e57805c7745b5259a9b1caaa115768ec225452f12f354e550f83e071f9c6fee2c36698b4679191895aab8de
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 18, 2020
125f4a4 [tests] Require all tests to follow naming convention (Anthony Towns)

Pull request description:

  Based on top of bitcoin#11774

Tree-SHA512: 1eb156b5a97b30c203b7b0ad3d2055b391ef825e2e57805c7745b5259a9b1caaa115768ec225452f12f354e550f83e071f9c6fee2c36698b4679191895aab8de
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants