Skip to content

Commit

Permalink
Run tests with TSAN where available, add macOS CI for Swift 5.2 (#799)
Browse files Browse the repository at this point in the history
Motivation:

We should run TSAN in tests.

Modifications:

- run TSAN by default; don't run TSAN for Swift 5.0 on Linux
- add Xcode 11.4 to add CI for Swift 5.2 on macOS

Result:

Resolves #757
  • Loading branch information
glbrntt authored Jun 8, 2020
1 parent 0048e6b commit caaaa5f
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 13 deletions.
11 changes: 9 additions & 2 deletions .travis-script.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,15 @@ make_all() {

make_test() {
echo -en 'travis_fold:start:make.test\\r'
info "Running Swift tests"
make test

if [[ ${NO_TSAN} == "true" ]]; then
info "Running Swift tests"
make test
else
info "Running Swift tests with TSAN"
make test-tsan
fi

success "Swift tests passed"
echo -en 'travis_fold:end:make.test\\r'
}
Expand Down
14 changes: 12 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,24 @@ jobs:
env: RUN_INTEROP_TESTS=false SWIFT_VERSION=5.2
- <<: *tests
name: "Unit Tests: Ubuntu 18.04 (Swift 5.1)"
env: RUN_INTEROP_TESTS=false SWIFT_VERSION=5.1.5
env: RUN_INTEROP_TESTS=false SWIFT_VERSION=5.1.5 NO_TSAN=true
- <<: *tests
name: "Unit Tests: Ubuntu 18.04 (Swift 5.0)"
env: RUN_INTEROP_TESTS=false SWIFT_VERSION=5.0.3
env: RUN_INTEROP_TESTS=false SWIFT_VERSION=5.0.3 NO_TSAN=true
- <<: *tests
name: "Unit Tests: Xcode 11.4 (Swift 5.2)"
os: osx
osx_image: xcode11.4
- <<: *tests
name: "Unit Tests: Xcode 11.3 (Swift 5.1)"
os: osx
osx_image: xcode11.3
env: NO_TSAN=true
- <<: *tests
name: "Unit Tests: Xcode 10.3 (Swift 5.0)"
os: osx
osx_image: xcode10.3
env: NO_TSAN=true
# Interop Tests.
- &interop_tests
stage: "Interoperability Tests"
Expand All @@ -55,6 +61,10 @@ jobs:
- <<: *interop_tests
name: "Interoperability Tests: Ubuntu 18.04 (Swift 5.0)"
env: RUN_INTEROP_TESTS=true SWIFT_VERSION=5.0.3
- <<: *interop_tests
name: "Interoperability Tests: Xcode 11.4 (Swift 5.2)"
os: osx
osx_image: xcode11.4
- <<: *interop_tests
name: "Interoperability Tests: Xcode 11.3 (Swift 5.1)"
os: osx
Expand Down
5 changes: 5 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ generate-route-guide: ${ROUTE_GUIDE_PB} ${ROUTE_GUIDE_GRPC}
test:
${SWIFT_TEST}

# Normal test suite with TSAN enabled.
.PHONY:
test-tsan:
${SWIFT_TEST} --sanitize=thread

# Checks that linuxmain has been updated: requires macOS.
.PHONY:
test-generate-linuxmain: generate-linuxmain
Expand Down
4 changes: 3 additions & 1 deletion Tests/GRPCTests/BasicEchoTestCase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ enum TransportSecurity {
}

class EchoTestCaseBase: GRPCTestCase {
var defaultTestTimeout: TimeInterval = 1.0
// Things can be slow when running under TSAN; bias towards a really long timeout so that we know
// for sure a test is wedged rather than simply slow.
var defaultTestTimeout: TimeInterval = 120.0

var serverEventLoopGroup: EventLoopGroup!
var clientEventLoopGroup: EventLoopGroup!
Expand Down
8 changes: 2 additions & 6 deletions Tests/GRPCTests/FunctionalTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class FunctionalTestsInsecureTransport: EchoTestCaseBase {

// Instead of setting a timeout out on the test we'll set one for each batch, if any of them
// timeout then we'll bail out of the test.
let batchTimeout: TimeInterval = 5.0
let batchTimeout: TimeInterval = 30.0
self.continueAfterFailure = false

for lowerBound in stride(from: 0, to: numberOfRequests, by: batchSize) {
Expand Down Expand Up @@ -133,7 +133,6 @@ class FunctionalTestsInsecureTransport: EchoTestCaseBase {

func testClientStreamingLotsOfMessages() throws {
guard self.runTimeSensitiveTests() else { return }
self.defaultTestTimeout = 15.0
XCTAssertNoThrow(try doTestClientStreaming(messages: lotsOfStrings))
}

Expand Down Expand Up @@ -161,7 +160,6 @@ class FunctionalTestsInsecureTransport: EchoTestCaseBase {

func testServerStreamingLotsOfMessages() {
guard self.runTimeSensitiveTests() else { return }
self.defaultTestTimeout = 15.0
XCTAssertNoThrow(try doTestServerStreaming(messages: lotsOfStrings))
}

Expand All @@ -186,7 +184,7 @@ class FunctionalTestsInsecureTransport: EchoTestCaseBase {

messages.forEach { part in
call.sendMessage(Echo_EchoRequest(text: part), promise: nil)
XCTAssertNotEqual(responseReceived?.wait(timeout: .now() + .seconds(1)), .some(.timedOut), line: line)
XCTAssertNotEqual(responseReceived?.wait(timeout: .now() + .seconds(30)), .some(.timedOut), line: line)
}
call.sendEnd(promise: nil)

Expand All @@ -203,13 +201,11 @@ class FunctionalTestsInsecureTransport: EchoTestCaseBase {

func testBidirectionalStreamingLotsOfMessagesBatched() throws {
guard self.runTimeSensitiveTests() else { return }
self.defaultTestTimeout = 15.0
XCTAssertNoThrow(try doTestBidirectionalStreaming(messages: lotsOfStrings))
}

func testBidirectionalStreamingLotsOfMessagesPingPong() throws {
guard self.runTimeSensitiveTests() else { return }
self.defaultTestTimeout = 15.0
XCTAssertNoThrow(try doTestBidirectionalStreaming(messages: lotsOfStrings, waitForEachResponse: true))
}
}
Expand Down
6 changes: 4 additions & 2 deletions Tests/GRPCTests/ServerThrowingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ extension ServerThrowingTests {

func testClientStreaming() throws {
let call = client.collect()
XCTAssertNoThrow(try call.sendEnd().wait())
// This is racing with the server error; it might fail, it might not.
try? call.sendEnd().wait()
XCTAssertEqual(expectedError, try call.status.wait())

if type(of: makeEchoProvider()) != ErrorReturningEchoProvider.self {
Expand All @@ -150,7 +151,8 @@ extension ServerThrowingTests {

func testBidirectionalStreaming() throws {
let call = client.update() { XCTFail("no message expected, got \($0)") }
XCTAssertNoThrow(try call.sendEnd().wait())
// This is racing with the server error; it might fail, it might not.
try? call.sendEnd().wait()
// Nothing to throw here, but the `status` should be the expected error.
XCTAssertEqual(expectedError, try call.status.wait())
}
Expand Down

0 comments on commit caaaa5f

Please sign in to comment.