Skip to content

Make always_run test decorator a tag and improve shard tests. - #8675

Merged
sklam merged 5 commits into
numba:mainfrom
stuartarchibald:fix/8657
Jan 11, 2023
Merged

sklam merged 5 commits into
numba:mainfrom
stuartarchibald:fix/8657

Conversation

@stuartarchibald

Copy link
Copy Markdown
Contributor

This patch:

  • Makes an always_run test decorator to use on tests that should run in all test shards, it's based on the existing test tag mechanism.
  • Updates the test runner to recognise the always_run tag when sharding so as to ensure that tests tagged with this tag will always be included in a shard.
  • Updates the unit tests to ensure fundamental properties with regards to sharding. This includes shard uniqueness, the presence of the always_run tests in shards and also that the sum of the test shards and always_run tests is the same as the full test listing.
  • Fixes up a few references slice->shard.

Fixes #8657

Comment on lines +169 to +187
# check that the always running tests are in every shard, and then
# remove them from the shards
for shard in sharded_sets:
for test in always_running:
self.assertIn(test, shard)
shard.remove(test)
self.assertNotIn(test, shard)

# check that there is no overlap between the shards
for a, b in itertools.combinations(sharded_sets, 2):
self.assertFalse(a & b)

# check that the sum of the shards and the always running tests is the
# same as the full listing

sum_of_parts = set()
[sum_of_parts.update(x) for x in sharded_sets]
sum_of_parts.update(always_running)

full_listing = set(self._get_numba_tests_from_listing(
self.get_testsuite_listing([])))

self.assertEqual(sum_of_parts, full_listing)

@stuartarchibald stuartarchibald Dec 16, 2022 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is quite verbose but with the intention of being clear. Another option might be to just compute the intersection of each combination of shards and make sure that it matches the set of "always_running" tests, then combine all shards and seeing if they match the full listing.

Comment thread numba/tests/test_runtests.py Outdated
This patch:

* Makes an `always_run` test decorator to use on tests that should
  run in all test shards, it's based on the existing test `tag`
  mechanism.
* Updates the test runner to recognise the `always_run` tag when
  sharding so as to ensure that tests tagged with this tag will
  always be included in a shard.
* Updates the unit tests to ensure fundamental properties with
  regards to sharding. This includes shard uniqueness, the presence
  of the `always_run` tests in shards and also that the sum of the
  test shards and `always_run` tests is the same as the full test
  listing.
* Fixes up a few references slice->shard.

Fixes numba#8657
Comment thread numba/tests/test_runtests.py Outdated
Comment thread numba/tests/test_runtests.py Outdated
Resolved conflicts in:
	buildscripts/azure/azure-windows.yml
@stuartarchibald

Copy link
Copy Markdown
Contributor Author

@apmasell thanks for the review. Note that since review:

  • 3772028 merges main and fixes conflicts.
  • 228112a fixes a comment slice->shard.
  • 47be4be changes list comp to loops as per review comment.

@stuartarchibald
stuartarchibald marked this pull request as ready for review January 11, 2023 10:10
@stuartarchibald stuartarchibald added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 2 - In Progress labels Jan 11, 2023
@stuartarchibald stuartarchibald added this to the Numba 0.57 RC milestone Jan 11, 2023
As title. Other testing in this module asserts that the shards are
of a suitable size.
@stuartarchibald stuartarchibald added 4 - Waiting on second reviewer Patch needs a second reviewer. and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Jan 11, 2023
@sklam sklam added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on second reviewer Patch needs a second reviewer. labels Jan 11, 2023
@sklam
sklam merged commit fb41749 into numba:main Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to merge Review and testing done, is ready to merge Effort - medium Medium size effort needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check test shards do not accidentally overlap

3 participants