Skip to content

Commit 94f2eb4

Browse files
authored
[scalatest] More conservative testdir mangling (#4975)
Change the way that test directory names are mangled to be much more conservative. Mangle any character which isn't a letter, number, underscore/dash, or period. This was empirically observed to be needed for Verilator v5.038 by @ngraybeal. Signed-off-by: Schuyler Eldridge <[email protected]>
1 parent 1a2980c commit 94f2eb4

File tree

2 files changed

+15
-14
lines changed

2 files changed

+15
-14
lines changed

src/main/scala/chisel3/testing/scalatest/WithTestingDirectory.scala

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,20 +53,21 @@ trait TestingDirectory extends TestSuiteMixin { self: TestSuite =>
5353
*/
5454
final implicit def implementation: HasTestingDirectory = new HasTestingDirectory {
5555

56-
// A sequence of regular expressions and their replacements that should be
57-
// applied to the test name.
58-
val res = Seq("\\s|\\(|\\)|\\$|/|\\\\".r -> "-", "\"|\'|#|:|;|<|>".r -> "")
56+
// Match non-simple characters.
57+
val regex = "[^-_.A-Za-z0-9]".r
5958

60-
/** Return the test name with minimal sanitization applied:
59+
/** Return the test name with overly conservative sanitization applied.
6160
*
62-
* - Replace all whitespace as this is incompatible with GNU make [1]
63-
* - Replace any characters which Verilator Makefiles empirically have
64-
* problems with
61+
* This should narrowly only need to replace whitespace (as this is
62+
* incompatible with GNU make [1] and any character which Verilator
63+
* empirically has problems with). However, this has historically been a
64+
* game of whack-a-mole and hence this replaces much more than it likely
65+
* needs to.
6566
*
6667
* [1]: https://savannah.gnu.org/bugs/?712
6768
*/
68-
final def getTestName = testName.value.map { case a =>
69-
res.foldLeft(a) { case (string, (regex, replacement)) => regex.replaceAllIn(string, replacement) }
69+
final def getTestName = testName.value.map { case name =>
70+
regex.replaceAllIn(name, _ => "-")
7071
}
7172

7273
override def getDirectory: Path = FileSystems

src/test/scala-2/chiselTests/testing/scalatest/TestingDirectorySpec.scala

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,27 +64,27 @@ class TestingDirectorySpec extends AnyFunSpec with Matchers with SimulatorAPI wi
6464
"chiselsim",
6565
"TestingDirectorySpec",
6666
"A-test-suite-mixing-in-WithTestingDirectory",
67-
"should-generate-another-directory,-too"
67+
"should-generate-another-directory--too"
6868
)
6969
}
7070

71-
it("should handle emojis, e.g., 🚀") {
71+
it("should mangle emojis, e.g., 🚀") {
7272
checkDirectoryStructure(
7373
"build",
7474
"chiselsim",
7575
"TestingDirectorySpec",
7676
"A-test-suite-mixing-in-WithTestingDirectory",
77-
"should-handle-emojis,-e.g.,-🚀"
77+
"should-mangle-emojis--e.g.---"
7878
)
7979
}
8080

81-
it("should handle CJK characters, e.g., 好猫咪") {
81+
it("should mangle CJK characters, e.g., 好猫咪") {
8282
checkDirectoryStructure(
8383
"build",
8484
"chiselsim",
8585
"TestingDirectorySpec",
8686
"A-test-suite-mixing-in-WithTestingDirectory",
87-
"should-handle-CJK-characters,-e.g.,-好猫咪"
87+
"should-mangle-CJK-characters--e.g.-----"
8888
)
8989
}
9090

0 commit comments

Comments
 (0)