runner: Self-check tests more accurately and earlier.
We didn't correctly handle tests where the versions figure into
resumeConfig and got by because the test didn't actually check the
version. Run it more accurately, and check more fields. Also add a
skipVersionNameCheck option for when the heuristic doesn't work.
(Some of the tests specify a TLS maximum version by passing in all the
-no-tls1, etc., flags for the other versions. Moreover, some of them
will set no flags for a maximum of TLS 1.3. Suppress the check on those
tests.)
Change-Id: I72c069b1baed09e29bf502036957fe0e90edbe60
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48000
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 3ab350d..f2ad64f 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -696,6 +696,9 @@
// skipSplitHandshake, if true, will skip the generation of a split
// handshake copy of the test.
skipSplitHandshake bool
+ // skipVersionNameCheck, if true, will skip the consistency check between
+ // test name and the versions.
+ skipVersionNameCheck bool
}
var testCases []testCase
@@ -1290,24 +1293,6 @@
}
}()
- if !test.shouldFail && (len(test.expectedError) > 0 || len(test.expectedLocalError) > 0) {
- panic("Error expected without shouldFail in " + test.name)
- }
-
- if test.expectResumeRejected && !test.resumeSession {
- panic("expectResumeRejected without resumeSession in " + test.name)
- }
-
- for _, ver := range tlsVersions {
- if !strings.Contains("-"+test.name+"-", "-"+ver.name+"-") {
- continue
- }
-
- if test.config.MaxVersion == 0 && test.config.MinVersion == 0 && test.expectations.version == 0 {
- panic(fmt.Sprintf("The name of test %q suggests that it's version specific, but min/max version in the Config is %x/%x. One of them should probably be %x", test.name, test.config.MinVersion, test.config.MaxVersion, ver.version))
- }
- }
-
listener, err := net.ListenTCP("tcp", &net.TCPAddr{IP: net.IPv6loopback})
if err != nil {
listener, err = net.ListenTCP("tcp4", &net.TCPAddr{IP: net.IP{127, 0, 0, 1}})
@@ -6142,6 +6127,9 @@
expectations: connectionExpectations{
version: expectedVersion,
},
+ // The version name check does not recognize the
+ // |excludeFlag| construction in |flags|.
+ skipVersionNameCheck: true,
})
testCases = append(testCases, testCase{
protocol: protocol,
@@ -6173,6 +6161,9 @@
expectations: connectionExpectations{
version: expectedVersion,
},
+ // The version name check does not recognize the
+ // |excludeFlag| construction in |flags|.
+ skipVersionNameCheck: true,
})
testCases = append(testCases, testCase{
protocol: protocol,
@@ -6567,6 +6558,9 @@
shouldFail: shouldFail,
expectedError: expectedError,
expectedLocalError: expectedLocalError,
+ // The version name check does not recognize the
+ // |excludeFlag| construction in |flags|.
+ skipVersionNameCheck: true,
})
testCases = append(testCases, testCase{
protocol: protocol,
@@ -6605,6 +6599,9 @@
shouldFail: shouldFail,
expectedError: expectedError,
expectedLocalError: expectedLocalError,
+ // The version name check does not recognize the
+ // |excludeFlag| construction in |flags|.
+ skipVersionNameCheck: true,
})
testCases = append(testCases, testCase{
protocol: protocol,
@@ -13079,6 +13076,7 @@
MaxVersion: VersionTLS12,
SessionTicketsDisabled: true,
},
+ flags: []string{"-max-version", strconv.Itoa(VersionTLS13)},
resumeSession: true,
})
@@ -17523,6 +17521,44 @@
return matched, nil
}
+func checkTests() {
+ for _, test := range testCases {
+ if !test.shouldFail && (len(test.expectedError) > 0 || len(test.expectedLocalError) > 0) {
+ panic("Error expected without shouldFail in " + test.name)
+ }
+
+ if test.expectResumeRejected && !test.resumeSession {
+ panic("expectResumeRejected without resumeSession in " + test.name)
+ }
+
+ if !test.skipVersionNameCheck {
+ for _, ver := range tlsVersions {
+ if !strings.Contains("-"+test.name+"-", "-"+ver.name+"-") {
+ continue
+ }
+
+ found := test.config.MaxVersion == ver.version || test.config.MinVersion == ver.version || test.expectations.version == ver.version
+ if test.resumeConfig != nil {
+ found = found || test.resumeConfig.MaxVersion == ver.version || test.resumeConfig.MinVersion == ver.version
+ }
+ if test.resumeExpectations != nil {
+ found = found || test.resumeExpectations.version == ver.version
+ }
+ shimFlag := ver.shimFlag(test.protocol)
+ for _, flag := range test.flags {
+ if flag == shimFlag {
+ found = true
+ break
+ }
+ }
+ if !found {
+ panic(fmt.Sprintf("The name of test %q suggests that it's version specific, but the test does not reference %s", test.name, ver.name))
+ }
+ }
+ }
+ }
+}
+
func main() {
flag.Parse()
*resourceDir = path.Clean(*resourceDir)
@@ -17599,7 +17635,7 @@
}
testCases = append(testCases, toAppend...)
- var wg sync.WaitGroup
+ checkTests()
numWorkers := *numWorkersFlag
if useDebugger() {
@@ -17612,6 +17648,7 @@
go statusPrinter(doneChan, statusChan, len(testCases))
+ var wg sync.WaitGroup
for i := 0; i < numWorkers; i++ {
wg.Add(1)
go worker(statusChan, testChan, *shimPath, &wg)
