Bug 188261

Summary:	run-api-tests: It is not very meaningful to prepend the binary name to tests
Product:	WebKit	Reporter:	Daniel Bates <dbates>
Component:	Tools / Tests	Assignee:	Nobody <webkit-unassigned>
Status:	NEW ---
Severity:	Normal	CC:	ap, jbedard, lforschler
Priority:	P2
Version:	WebKit Local Build
Hardware:	All
OS:	All
See Also:	https://bugs.webkit.org/show_bug.cgi?id=187893

Description Daniel Bates 2018-08-02 10:17:21 PDT

Following <https://trac.webkit.org/changeset/230998> (bug #181043), run-api-tests now prepends the binary name of the test executable that contains the test when it emits the name of the test. Knowing the binary name that contains a test does not seem very meaningful because it is an implementation detail of how we chose to bundle and run tests (by splitting tests across more than one binary). As far as I can tell, any advantage claimed by prepending the binary name to the test to avoid collisions seem superfluous. It would be good to know if people ever ran into such collisions. Otherwise, it seems sufficient to revert to our behavior before <https://trac.webkit.org/changeset/230998> and rely on Google unit test's default namespace behavior where each test is identified by its test suite and test name.

This problem may be easier to see visually. The following is the output of a passing test and failing test run with run-api-test before and after r230998.

== Passing test ==

Before r230998:

[[
$ Tools/Scripts/old-run-api-tests --debug --no-build --verbose WTF.StringViewStripLeadingAndTrailingMatchedCharacters
PASS WTF.StringViewStripLeadingAndTrailingMatchedCharacters
]]

After r230998:

[[
$ Tools/Scripts/run-api-tests --debug --no-build --verbose WTF.StringViewStripLeadingAndTrailingMatchedCharacters
Checking build ...
"perl Tools/Scripts/webkit-build-directory --configuration --debug --mac" took 0.41s
Collecting tests ...
"/Volumes/Data/WebKitDev/OpenSource/WebKitBuild/Debug/TestWTF --gtest_list_tests" took 0.16s
"/Volumes/Data/WebKitDev/OpenSource/WebKitBuild/Debug/TestWebKitAPI --gtest_list_tests" took 0.23s
Found 1 tests
Running tests
Sharding tests ...
worker/0 starting
TestWTF.WTF.StringViewStripLeadingAndTrailingMatchedCharacters Passed
worker/0 exiting
Ran 1 tests of 1 with 1 successful
------------------------------
All tests successfully passed!
Testing completed, Exit status: 0
]]

== Failing test ==

Before r230998:

[[
$ Tools/Scripts/old-run-api-tests --debug --no-build --verbose WTF.StringViewStripLeadingAndTrailingMatchedCharacters
FAIL WTF.StringViewStripLeadingAndTrailingMatchedCharacters

/Volumes/Data/WebKitDev/OpenSource/Tools/TestWebKitAPI/Tests/WTF/StringView.cpp:958
Value of: stringViewFromLiteral("AAABBAAA").stripLeadingAndTrailingMatchedCharacters(isA) == stringViewFromLiteral("BBB")
  Actual: false
Expected: true


Tests that failed:
  WTF.StringViewStripLeadingAndTrailingMatchedCharacters
]]

After r230998:

[[
$ Tools/Scripts/run-api-tests --debug --no-build --verbose WTF.StringViewStripLeadingAndTrailingMatchedCharacters
Checking build ...
"perl Tools/Scripts/webkit-build-directory --configuration --debug --mac" took 0.39s
Collecting tests ...
"/Volumes/Data/WebKitDev/OpenSource/WebKitBuild/Debug/TestWTF --gtest_list_tests" took 0.17s
"/Volumes/Data/WebKitDev/OpenSource/WebKitBuild/Debug/TestWebKitAPI --gtest_list_tests" took 0.20s
Found 1 tests
Running tests
Sharding tests ...
worker/0 starting
    
    /Volumes/Data/WebKitDev/OpenSource/Tools/TestWebKitAPI/Tests/WTF/StringView.cpp:958
    Value of: stringViewFromLiteral("AAABBAAA").stripLeadingAndTrailingMatchedCharacters(isA) == stringViewFromLiteral("BBB")
      Actual: false
    Expected: true
    
TestWTF.WTF.StringViewStripLeadingAndTrailingMatchedCharacters Failed
worker/0 exiting
Ran 1 tests of 1 with 0 successful
------------------------------
Test suite failed

Failed

    TestWTF.WTF.StringViewStripLeadingAndTrailingMatchedCharacters
        
        /Volumes/Data/WebKitDev/OpenSource/Tools/TestWebKitAPI/Tests/WTF/StringView.cpp:958
        Value of: stringViewFromLiteral("AAABBAAA").stripLeadingAndTrailingMatchedCharacters(isA) == stringViewFromLiteral("BBB")
          Actual: false
        Expected: true
        

Testing completed, Exit status: 3
]]

As you can see from the above output we did not prepend the binary name (TestWTF) to the test name before r230998.

Comment 1 Jonathan Bedard 2018-08-02 11:59:15 PDT

I very strongly disagree with the premise of this bug.

Prepending the binary name ensures that we won't have collisions, and it really doesn't do any harm. This bug would have more merit if the test harness didn't recognize a test by <suite>.<name>, but it does.

Additionally, if we start tracking API test results (which is something that's been requested) colliding suite or test names would be a real problem.