Skip to content

Commit

Permalink
Fix division by zero when sharding is disabled but test filtering is …
Browse files Browse the repository at this point in the history
…used.

Also allow the test filter to match any part of the test name, not just the whole string. Users can explicitly write `^...$` to force a whole string match.

PiperOrigin-RevId: 607074089
  • Loading branch information
allevato authored and swiple-rules-gardener committed Feb 14, 2024
1 parent 80cdccc commit 1b1f348
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 3 deletions.
2 changes: 1 addition & 1 deletion tools/test_discoverer/ObjcTestPrinter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ struct ObjcTestPrinter {
guard isIncludedByFilter(nameForFiltering(of: test)) else {
break
}
if seenTestCount % shardCount == shardIndex {
if isIncludedInShard() {
shardedSuite.addTest(test)
}
seenTestCount += 1
Expand Down
2 changes: 1 addition & 1 deletion tools/test_discoverer/SymbolGraphTestPrinter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ struct SymbolGraphTestPrinter {
guard isIncludedByFilter("\\(suiteName)/\\(test.0)") else {
continue
}
if seenTestCount % shardCount == shardIndex {
if isIncludedInShard() {
shardTests.append(test)
}
seenTestCount += 1
Expand Down
6 changes: 5 additions & 1 deletion tools/test_discoverer/TestPrinterCommon.swift
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,15 @@ func createShardingFilteringTestCollector(extraProperties: String = "") -> Strin
private func isIncludedByFilter(_ testName: String) -> Bool {
guard let filter = self.filter else { return true }
do {
return try filter.wholeMatch(in: testName) != nil
return try filter.firstMatch(in: testName) != nil
} catch {
return false
}
}
private func isIncludedInShard() -> Bool {
return shardCount == 0 || seenTestCount % shardCount == shardIndex
}
}
"""
Expand Down

1 comment on commit 1b1f348

@brentleyjones
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please sign in to comment.