Infer test group name directly from test file path #3418
Conversation
test/test_runner/test_parser.cpp
Outdated
| @@ -67,8 +79,6 @@ void TestParser::parseHeader() { | |||
| break; | |||
| } | |||
| case TokenType::GROUP: { | |||
There was a problem hiding this comment.
Do we still need TokenType::GROUP?
There was a problem hiding this comment.
I kept it in for the sake of backward compatibility. We can remove it if we want to.
There was a problem hiding this comment.
I think we can just remove it then.
test/test_runner/test_parser.cpp
Outdated
| std::size_t subEnd = path.find_last_of('.') - 1; | ||
| std::string relPath = path.substr(subStart, subEnd - subStart + 1); | ||
| std::replace(relPath.begin(), relPath.end(), '-', '_'); | ||
| std::replace(relPath.begin(), relPath.end(), '/', '|'); |
There was a problem hiding this comment.
hmm will this replace work on windows?
There was a problem hiding this comment.
added another replace line for windows backslash. Also had to change delimiter since windows does not support |
test/test_runner/test_parser.cpp
Outdated
| 1; | ||
| std::size_t subEnd = path.find_last_of('.') - 1; | ||
| std::string relPath = path.substr(subStart, subEnd - subStart + 1); | ||
| std::replace(relPath.begin(), relPath.end(), '-', '_'); |
There was a problem hiding this comment.
I don't think we should replace - with _. Users of the testing framework should follow the convention of using _ instead of - in file/dir names themselves.
There was a problem hiding this comment.
I did this as a workaround as a bunch of files/directories had - in the codebase which results in a false positive during the test runs. I could also go and change those files and remove this line.
There was a problem hiding this comment.
What do u mean "false positive" here?
There was a problem hiding this comment.
Removed this line and fixed the naming of existing tests
test/test_runner/test_parser.cpp
Outdated
| @@ -67,8 +79,6 @@ void TestParser::parseHeader() { | |||
| break; | |||
| } | |||
| case TokenType::GROUP: { | |||
There was a problem hiding this comment.
I think we can just remove it then.
|
Also, can u revisit the false positive issue? We'd better understand what's acceptable directory and file naming conventions under |
yiyun-sj
force-pushed
the
test-group-generation
branch
from
06c8432
to
83aa526
Compare
Test Group Name Generation
-GROUPproperty is not removed for backward compatibility (is this necessary?)Note: some characters such as : and - result in tests not being actually run (some current test cases are like this)
Example
the file
test/test_files/tck/match/match8now has test group nametck~match~match8Todo
delete
-GROUPfrom all existing test files (do we want to do this?)