Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tests] Rename functional tests #11774

Merged
merged 6 commits into from Jan 25, 2018
Merged

[tests] Rename functional tests #11774

merged 6 commits into from Jan 25, 2018

Conversation

ajtowns
Copy link
Contributor

@ajtowns ajtowns commented Nov 27, 2017

This PR changes the functional tests to have a consistent naming scheme:

tests for individual RPC methods are named rpc_...
tests for interfaces (REST, ZMQ, RPC features) are named interface_...
tests that explicitly test the p2p interface are named p2p_...
tests for wallet features are named wallet_...
tests for mining features are named mining_...
tests for mempool behaviour are named mempool_...
tests for full features that aren't wallet/mining/mempool are named feature_...

Rationale: it's sometimes difficult for new contributors to know what's already covered by existing tests and where new tests should be added. Naming in a consistent fashion makes it easier to see what's already covered at a glance.

@fanquake fanquake added the Tests label Nov 27, 2017
@ajtowns ajtowns changed the title [tests] Rename functional tests [WIP] [tests] Rename functional tests Nov 30, 2017
@ajtowns
Copy link
Contributor Author

ajtowns commented Nov 30, 2017

Split into two PRs -- changes in #11796 should be reviewed and merged first. This PR will just be the actual renaming, so is easy to automatically regenerate, and should be easy to review once the previous PR is merged. Have also split the renaming into multiple commits by test area, which I think makes it a bit easier to review, but happy to revert that if not.

Marking as WIP until #11796 is actually merged, since I'm expecting this to require a bunch of rebases.

@ajtowns ajtowns force-pushed the rename_tests branch 2 times, most recently from 15d692d to 08ea198 Compare December 5, 2017 08:29
@ajtowns
Copy link
Contributor Author

ajtowns commented Dec 5, 2017

Updated to match fixes in parent PR, rebased to current master to fix merge conflicts, added final commit doing away with EXPECTED_VIOLATION_COUNT (but still allowing tests to pass with a warning if there's 10 or fewer violations)

@ajtowns ajtowns force-pushed the rename_tests branch 2 times, most recently from 42588e5 to d522a5b Compare December 7, 2017 18:15
@ajtowns ajtowns force-pushed the rename_tests branch 2 times, most recently from 9bae101 to 838ddf1 Compare December 13, 2017 03:18
@ajtowns ajtowns force-pushed the rename_tests branch 2 times, most recently from af48e03 to aeb35d9 Compare January 7, 2018 21:17
@ajtowns ajtowns force-pushed the rename_tests branch 2 times, most recently from 89ffd28 to 75ee34c Compare January 12, 2018 01:48
laanwj added a commit that referenced this pull request Jan 15, 2018
5fecd84 [tests] Remove redundant import in blocktools.py test (Anthony Towns)
9b20bb4 [tests] Check tests conform to naming convention (Anthony Towns)
7250b4e [tests] README.md nit fixes (Anthony Towns)
82b2712 [tests] move witness util functions to blocktools.py (John Newbery)
1e10854 [tests] [docs] update README for new test naming scheme (John Newbery)

Pull request description:

  Splitting #11774 into two parts -- this part updates the README with the proposed naming convention, and adds some checks to test_runner.py that the number of tests violating the naming convention doesn't increase too much. Idea is this part of the change should not introduce merge conflicts or require much rebasing, so reviews of the complicated bits won't become invalidated too often; while the second part will just be file renames, which will require regular rebasing and will introduce merge conflicts with pending PRs, but can be merged later, and should also be much easier to review, since it will only include relatively trivial changes.

Tree-SHA512: b96557d41714addbbfe2aed62fb5a48639eaeb1eb3aba30ac1b3a86bb3cb8d796c6247f9c414c4695c4bf54c0ec9968ac88e2f88fb62483bc1a2f89368f7fc80
@maflcko
Copy link
Member

maflcko commented Jan 15, 2018

@ajtowns Is this still WIP?

@jnewbery
Copy link
Contributor

Tested ACK: c3fd1259f14e513934f6b2eee1f7febe708d04c7

@ajtowns
Copy link
Contributor Author

ajtowns commented Jan 15, 2018

@MarcoFalke if it's merged, I think github will decide that any other PRs that touch any of the renamed test cases have conflicts and can't be automatically merged. I was going to put together a list of those PRs before removing the WIP tag, but it's late now, so tomorrow.

@ajtowns
Copy link
Contributor Author

ajtowns commented Jan 16, 2018

The following PRs have the 0.16.0 milestone and touch test cases:

I think github will decide there's conflicts and a rebase is required in whichever of these PRs get merged later (ie if 12119, 12101 or 11904 is merged first, github will claim this PR has conflicts; if this one is merged first, github will claim those PRs have conflicts).

There's a few PRs marked 0.17.0 or Future with the same issue along with some PRs that already have merge conflicts according to github. That leaves the following PRs that will likely "conflict" with this PR, that don't have a milestone and are currently mergeable:

@maflcko
Copy link
Member

maflcko commented Jan 17, 2018

I think it makes sense to merge this into the 0.16 branch, so it is easier to switch back and forth between last release and master and have the same name for tests. I'd propose to wait for the merge of #12101 and #12119, then rebase on master and merge.

@ajtowns
Copy link
Contributor Author

ajtowns commented Jan 23, 2018

Rebased. Will still conflict with #12213 (which already needs a rebase) and #12119 so either this will need to be updated after they're merged, or they will need updating after this is merged.

@ajtowns ajtowns changed the title [WIP] [tests] Rename functional tests [tests] Rename functional tests Jan 23, 2018
@jnewbery
Copy link
Contributor

This should have a 0.16 milestone.

I think it's also fine to merge before #12213 and #12119 since both of those have review comments or rebasing to do anyway.

test scripts don't start with one of the allowed name prefixes."""
EXPECTED_VIOLATION_COUNT = 77

# LEEWAY is provided as a transition measure, so that pull-requests
# that introduce new tests that don't conform with the naming
Copy link
Member

Choose a reason for hiding this comment

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

Might as well set LEEWAY to 0, as I don't see a need for this.

After all, we want travis to fail if some test is named inproperly. Otherwise, it would lead to unnecessary fixup pull requests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, you might be right -- no specific example comes to mind, but I'm still a bit hesistant about removing LEEWAY prior to 0.16 branching from master.

I've put a commit that drops LEEWAY in #12252 ; assuming you definitely want it and Travis is happy with it, can merge it into this PR; otherwise can just merge #12252 when 0.16 branches off?

@maflcko
Copy link
Member

maflcko commented Jan 24, 2018

@ajtowns Needs rebase

@ajtowns
Copy link
Contributor Author

ajtowns commented Jan 25, 2018

Rebased

@maflcko
Copy link
Member

maflcko commented Jan 25, 2018

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

re-ACK 6f881cc8809e2c0d0150c47494bc37f2eb05ec66
-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJaaTX3AAoJENLqSFDnUoslH1YQAJBMhrBIoF0AMKMW3lQN+h6U
mfv6jh7DZwWVLTkF2X98LUn3FHgPsDXDSAJtvfp3j4WYuWVeUryBqYR9P1WpWcMg
PYKgm/7NELtgImzpQ2FK3f/l5qCooZmjdNLG7y/JK2WUMJxZfLa8Z3nSmBQZipFf
zN41t7s2aXaeTM4pD2cZ6vS5cnzSXQMN3WrUYakEnMG36Jf74hB2l0aOGAIlZixa
EMVzlult9sqVFyu3i5LzjLyU5MMy1mnQtH+cCjB0ScVTntcMMQhReLMvj1Uofa3z
9n+CO2B80CMZe+n7if4aW6bNPJbZw0aFjZ94nfOvwm3FHO8EZVZGVk1z/mOQIa1O
vmlAtFaD0N5pp4d0zjhPKcKGi4gs6G3beXo5p5/W9icbEMVBYhutroqxEO/Y6K9x
qIE9ZP2KhIiQLfogPP+RrkfOWWu6RwbIwf7pE1WLyGPAsFUBczmZvH/2tE1yhnUf
a9Pr+cOfeWkQAFXbftH0F750r/be/xKjhn56q1a6VG4QNcCUN0FSjRFLPrRn+1cD
Qxhl3Rnb/aKichY4kUnitvhfgALTkbFDKf5Brmh5seaif5qR+ujOewasGBc9We5s
YLDu/nyPuXcf5malXBudG5RL7Yuti/e1Y3vcGai4OTPniIg6d/9L880BJOgKJTsf
FnjxzQTjI02UBpO5VHFR
=XBcp
-----END PGP SIGNATURE-----

@maflcko maflcko merged commit 6f881cc into bitcoin:master Jan 25, 2018
maflcko pushed a commit that referenced this pull request Jan 25, 2018
6f881cc [tests] Remove EXPECTED_VIOLATION_COUNT (Anthony Towns)
3150b3f [tests] Rename misc functional tests. (Anthony Towns)
81b79f2 [tests] Rename rpc_* functional tests. (Anthony Towns)
61b8f7f [tests] Rename p2p_* functional tests. (Anthony Towns)
90600bc [tests] Rename wallet_* functional tests. (Anthony Towns)
ca6523d [tests] Rename feature_* functional tests. (Anthony Towns)

Pull request description:

  This PR changes the functional tests to have a consistent naming scheme:

      tests for individual RPC methods are named rpc_...
      tests for interfaces (REST, ZMQ, RPC features) are named interface_...
      tests that explicitly test the p2p interface are named p2p_...
      tests for wallet features are named wallet_...
      tests for mining features are named mining_...
      tests for mempool behaviour are named mempool_...
      tests for full features that aren't wallet/mining/mempool are named feature_...

  Rationale: it's sometimes difficult for new contributors to know what's already covered by existing tests and where new tests should be added. Naming in a consistent fashion makes it easier to see what's already covered at a glance.

Tree-SHA512: 4246790552d42bbd95f6d5bdf67702b81b3b2c583ce7eaf1fe6d8e254721279b47315973c6e9ae82dad6e4c747f12188160764bf2624c0f8f3b4d39330ec8b16
@jnewbery
Copy link
Contributor

Thanks for maintaining this PR @ajtowns . You're a patienter man than I!

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
laanwj added a commit that referenced this pull request Feb 6, 2018
8a6c62b [tests] Update README after filename change (Conor Scott)

Pull request description:

  Update test README after filename changes from #11774

Tree-SHA512: 2dd2e4d12e9e8bef4e76996f610aea758d221f8da31e14163168a6a0c635d32fc547542112d43c37fa165c289572b12798caf467fd933082f8eb129f8e5d6ca8
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 13, 2020
5fecd84 [tests] Remove redundant import in blocktools.py test (Anthony Towns)
9b20bb4 [tests] Check tests conform to naming convention (Anthony Towns)
7250b4e [tests] README.md nit fixes (Anthony Towns)
82b2712 [tests] move witness util functions to blocktools.py (John Newbery)
1e10854 [tests] [docs] update README for new test naming scheme (John Newbery)

Pull request description:

  Splitting bitcoin#11774 into two parts -- this part updates the README with the proposed naming convention, and adds some checks to test_runner.py that the number of tests violating the naming convention doesn't increase too much. Idea is this part of the change should not introduce merge conflicts or require much rebasing, so reviews of the complicated bits won't become invalidated too often; while the second part will just be file renames, which will require regular rebasing and will introduce merge conflicts with pending PRs, but can be merged later, and should also be much easier to review, since it will only include relatively trivial changes.

Tree-SHA512: b96557d41714addbbfe2aed62fb5a48639eaeb1eb3aba30ac1b3a86bb3cb8d796c6247f9c414c4695c4bf54c0ec9968ac88e2f88fb62483bc1a2f89368f7fc80
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 13, 2020
6f881cc [tests] Remove EXPECTED_VIOLATION_COUNT (Anthony Towns)
3150b3f [tests] Rename misc functional tests. (Anthony Towns)
81b79f2 [tests] Rename rpc_* functional tests. (Anthony Towns)
61b8f7f [tests] Rename p2p_* functional tests. (Anthony Towns)
90600bc [tests] Rename wallet_* functional tests. (Anthony Towns)
ca6523d [tests] Rename feature_* functional tests. (Anthony Towns)

Pull request description:

  This PR changes the functional tests to have a consistent naming scheme:

      tests for individual RPC methods are named rpc_...
      tests for interfaces (REST, ZMQ, RPC features) are named interface_...
      tests that explicitly test the p2p interface are named p2p_...
      tests for wallet features are named wallet_...
      tests for mining features are named mining_...
      tests for mempool behaviour are named mempool_...
      tests for full features that aren't wallet/mining/mempool are named feature_...

  Rationale: it's sometimes difficult for new contributors to know what's already covered by existing tests and where new tests should be added. Naming in a consistent fashion makes it easier to see what's already covered at a glance.

Tree-SHA512: 4246790552d42bbd95f6d5bdf67702b81b3b2c583ce7eaf1fe6d8e254721279b47315973c6e9ae82dad6e4c747f12188160764bf2624c0f8f3b4d39330ec8b16
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 13, 2020
5fecd84 [tests] Remove redundant import in blocktools.py test (Anthony Towns)
9b20bb4 [tests] Check tests conform to naming convention (Anthony Towns)
7250b4e [tests] README.md nit fixes (Anthony Towns)
82b2712 [tests] move witness util functions to blocktools.py (John Newbery)
1e10854 [tests] [docs] update README for new test naming scheme (John Newbery)

Pull request description:

  Splitting bitcoin#11774 into two parts -- this part updates the README with the proposed naming convention, and adds some checks to test_runner.py that the number of tests violating the naming convention doesn't increase too much. Idea is this part of the change should not introduce merge conflicts or require much rebasing, so reviews of the complicated bits won't become invalidated too often; while the second part will just be file renames, which will require regular rebasing and will introduce merge conflicts with pending PRs, but can be merged later, and should also be much easier to review, since it will only include relatively trivial changes.

Tree-SHA512: b96557d41714addbbfe2aed62fb5a48639eaeb1eb3aba30ac1b3a86bb3cb8d796c6247f9c414c4695c4bf54c0ec9968ac88e2f88fb62483bc1a2f89368f7fc80
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 13, 2020
6f881cc [tests] Remove EXPECTED_VIOLATION_COUNT (Anthony Towns)
3150b3f [tests] Rename misc functional tests. (Anthony Towns)
81b79f2 [tests] Rename rpc_* functional tests. (Anthony Towns)
61b8f7f [tests] Rename p2p_* functional tests. (Anthony Towns)
90600bc [tests] Rename wallet_* functional tests. (Anthony Towns)
ca6523d [tests] Rename feature_* functional tests. (Anthony Towns)

Pull request description:

  This PR changes the functional tests to have a consistent naming scheme:

      tests for individual RPC methods are named rpc_...
      tests for interfaces (REST, ZMQ, RPC features) are named interface_...
      tests that explicitly test the p2p interface are named p2p_...
      tests for wallet features are named wallet_...
      tests for mining features are named mining_...
      tests for mempool behaviour are named mempool_...
      tests for full features that aren't wallet/mining/mempool are named feature_...

  Rationale: it's sometimes difficult for new contributors to know what's already covered by existing tests and where new tests should be added. Naming in a consistent fashion makes it easier to see what's already covered at a glance.

Tree-SHA512: 4246790552d42bbd95f6d5bdf67702b81b3b2c583ce7eaf1fe6d8e254721279b47315973c6e9ae82dad6e4c747f12188160764bf2624c0f8f3b4d39330ec8b16
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 16, 2020
5fecd84 [tests] Remove redundant import in blocktools.py test (Anthony Towns)
9b20bb4 [tests] Check tests conform to naming convention (Anthony Towns)
7250b4e [tests] README.md nit fixes (Anthony Towns)
82b2712 [tests] move witness util functions to blocktools.py (John Newbery)
1e10854 [tests] [docs] update README for new test naming scheme (John Newbery)

Pull request description:

  Splitting bitcoin#11774 into two parts -- this part updates the README with the proposed naming convention, and adds some checks to test_runner.py that the number of tests violating the naming convention doesn't increase too much. Idea is this part of the change should not introduce merge conflicts or require much rebasing, so reviews of the complicated bits won't become invalidated too often; while the second part will just be file renames, which will require regular rebasing and will introduce merge conflicts with pending PRs, but can be merged later, and should also be much easier to review, since it will only include relatively trivial changes.

Tree-SHA512: b96557d41714addbbfe2aed62fb5a48639eaeb1eb3aba30ac1b3a86bb3cb8d796c6247f9c414c4695c4bf54c0ec9968ac88e2f88fb62483bc1a2f89368f7fc80
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 16, 2020
6f881cc [tests] Remove EXPECTED_VIOLATION_COUNT (Anthony Towns)
3150b3f [tests] Rename misc functional tests. (Anthony Towns)
81b79f2 [tests] Rename rpc_* functional tests. (Anthony Towns)
61b8f7f [tests] Rename p2p_* functional tests. (Anthony Towns)
90600bc [tests] Rename wallet_* functional tests. (Anthony Towns)
ca6523d [tests] Rename feature_* functional tests. (Anthony Towns)

Pull request description:

  This PR changes the functional tests to have a consistent naming scheme:

      tests for individual RPC methods are named rpc_...
      tests for interfaces (REST, ZMQ, RPC features) are named interface_...
      tests that explicitly test the p2p interface are named p2p_...
      tests for wallet features are named wallet_...
      tests for mining features are named mining_...
      tests for mempool behaviour are named mempool_...
      tests for full features that aren't wallet/mining/mempool are named feature_...

  Rationale: it's sometimes difficult for new contributors to know what's already covered by existing tests and where new tests should be added. Naming in a consistent fashion makes it easier to see what's already covered at a glance.

Tree-SHA512: 4246790552d42bbd95f6d5bdf67702b81b3b2c583ce7eaf1fe6d8e254721279b47315973c6e9ae82dad6e4c747f12188160764bf2624c0f8f3b4d39330ec8b16
UdjinM6 pushed a commit to dashpay/dash that referenced this pull request Jul 16, 2020
* Merge bitcoin#11796: [tests] Functional test naming convention

5fecd84 [tests] Remove redundant import in blocktools.py test (Anthony Towns)
9b20bb4 [tests] Check tests conform to naming convention (Anthony Towns)
7250b4e [tests] README.md nit fixes (Anthony Towns)
82b2712 [tests] move witness util functions to blocktools.py (John Newbery)
1e10854 [tests] [docs] update README for new test naming scheme (John Newbery)

Pull request description:

  Splitting bitcoin#11774 into two parts -- this part updates the README with the proposed naming convention, and adds some checks to test_runner.py that the number of tests violating the naming convention doesn't increase too much. Idea is this part of the change should not introduce merge conflicts or require much rebasing, so reviews of the complicated bits won't become invalidated too often; while the second part will just be file renames, which will require regular rebasing and will introduce merge conflicts with pending PRs, but can be merged later, and should also be much easier to review, since it will only include relatively trivial changes.

Tree-SHA512: b96557d41714addbbfe2aed62fb5a48639eaeb1eb3aba30ac1b3a86bb3cb8d796c6247f9c414c4695c4bf54c0ec9968ac88e2f88fb62483bc1a2f89368f7fc80

* update violation count

Signed-off-by: pasta <pasta@dashboost.org>

* Merge bitcoin#11774: [tests] Rename functional tests

6f881cc [tests] Remove EXPECTED_VIOLATION_COUNT (Anthony Towns)
3150b3f [tests] Rename misc functional tests. (Anthony Towns)
81b79f2 [tests] Rename rpc_* functional tests. (Anthony Towns)
61b8f7f [tests] Rename p2p_* functional tests. (Anthony Towns)
90600bc [tests] Rename wallet_* functional tests. (Anthony Towns)
ca6523d [tests] Rename feature_* functional tests. (Anthony Towns)

Pull request description:

  This PR changes the functional tests to have a consistent naming scheme:

      tests for individual RPC methods are named rpc_...
      tests for interfaces (REST, ZMQ, RPC features) are named interface_...
      tests that explicitly test the p2p interface are named p2p_...
      tests for wallet features are named wallet_...
      tests for mining features are named mining_...
      tests for mempool behaviour are named mempool_...
      tests for full features that aren't wallet/mining/mempool are named feature_...

  Rationale: it's sometimes difficult for new contributors to know what's already covered by existing tests and where new tests should be added. Naming in a consistent fashion makes it easier to see what's already covered at a glance.

Tree-SHA512: 4246790552d42bbd95f6d5bdf67702b81b3b2c583ce7eaf1fe6d8e254721279b47315973c6e9ae82dad6e4c747f12188160764bf2624c0f8f3b4d39330ec8b16

* rename tests and edit associated strings to align test-suite with test name standards

Signed-off-by: pasta <pasta@dashboost.org>

* fix grammar in test/functional/test_runner.py

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>

* ci: Fix excluded test names

* rename feature_privatesend.py to rpc_privatesend.py

Signed-off-by: pasta <pasta@dashboost.org>

Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>
Co-authored-by: xdustinface <xdustinfacex@gmail.com>
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
theStack added a commit to theStack/bitcoin that referenced this pull request Aug 15, 2021
The block test was renamed from `p2p-fullblocks.py` to
`feature_block.py` in commit ca6523d (PR bitcoin#11774).
@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.

None yet

5 participants