ci: run the test suite under AddressSanitizer (#487)

* ci: run the test suite under AddressSanitizer

The Test job becomes a two-entry matrix: the existing run, still named Test, and Test (ASan), the same xcodebuild invocation with -enableAddressSanitizer YES. Use-after-free and double-free bugs in the runtime are usually silent in a plain run, so a spec written for one (the nested-worker teardown spec from #479) passes there with or without its fix.

Artifact names are unique per workflow run, so the ASan entry uploads test-results-asan and test-diagnostics-asan, and collect-test-diagnostics takes the artifact name as an input.

Five worker quality-of-service specs cannot hold in an instrumented build: a worker asked for default or utility reads back user-initiated. They are reported as skipped there, keyed on a new TNSIsAddressSanitizerEnabled() test fixture, and run unchanged everywhere else.

* test: let the system assign the test server's port

The XCTest harness served the junit report and the HTTP module fixtures on a fixed loopback port. Simulators share the host's loopback interface, so two suite runs on one machine, even on different simulators, could deliver one run's report to the other's listener.

The listener now binds a port chosen by the system and hands it to the app through REPORT_BASEURL, which is already the only way the app learns the address. start() blocks until the listener is bound, since the port is only known then, and throws when binding fails instead of leaving the failure unobserved.

* test: log the test server's port

The port is assigned by the system at bind time, so the run's output is the only place to find it when poking at the server by hand.

* test: report test server startup errors as test failures

Setup moves to setUpWithError() so a listener that fails to bind, or does not become ready in time, is recorded by XCTest as a failure of the test instead of crashing the runner through try!. tearDown() tolerates a server that was never created, since it still runs after a throwing setup.

Author: edusperoni

.github/actions/collect-test-diagnostics/action.yml

@@ -9,6 +9,11 @@ inputs:
   test-folder:
     description: Folder the diagnostics directory is created in
     required: true
+  artifact-name:
+    description: >
+      Name of the uploaded artifact. Artifact names are unique per workflow
+      run, so jobs that can both fail in one run need distinct names.
+    default: test-diagnostics
 runs:
   using: composite
   steps:
@@ -45,6 +50,6 @@ runs:
     - name: Upload test diagnostics
       uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
       with:
-        name: test-diagnostics
+        name: ${{ inputs.artifact-name }}
         path: ${{ inputs.test-folder }}/diagnostics
         if-no-files-found: ignore

.github/workflows/pull_request.yml

@@ -56,9 +56,23 @@ jobs:
           name: NativeScript-dSYMs
           path: dist/dSYMs
   test:
-    name: Test
+    name: ${{ matrix.name }}
     runs-on: macos-15
     needs: build
+    strategy:
+      # The two variants answer different questions; one failing must not
+      # cancel the other.
+      fail-fast: false
+      matrix:
+        include:
+          - name: Test
+            xcodebuild-args: ""
+            artifact-suffix: ""
+          # Only code compiled here is instrumented: the runtime and the test
+          # fixtures, not the prebuilt V8 archives.
+          - name: Test (ASan)
+            xcodebuild-args: "-enableAddressSanitizer YES"
+            artifact-suffix: "-asan"
     steps:
       - uses: maxim-lobanov/setup-xcode@ed7a3b1fda3918c0306d1b724322adc0b8cc0a90 # v1.7.0
         with:
@@ -89,13 +103,13 @@ jobs:
           # TestRunnerTests.swift) need more than 20m headroom per attempt.
           timeout_minutes: 40
           max_attempts: 2
-          command: set -o pipefail && xcodebuild -project v8ios.xcodeproj -scheme TestRunner -resultBundlePath $TEST_FOLDER/test_results -destination platform\=iOS\ Simulator,OS\=latest,name\=iPhone\ 16\ Pro build test | xcpretty
+          command: set -o pipefail && xcodebuild -project v8ios.xcodeproj -scheme TestRunner -resultBundlePath $TEST_FOLDER/test_results -destination platform\=iOS\ Simulator,OS\=latest,name\=iPhone\ 16\ Pro ${{ matrix.xcodebuild-args }} build test | xcpretty
           # Keep the failed attempt's bundle — it holds the diagnostics of the
           # failure being retried. Everything else at the result path must go
           # (including extensionless staging leftovers), or the retry dies with
           # "Existing file at -resultBundlePath".
           on_retry_command: rm -rf $TEST_FOLDER/test_results_attempt1.xcresult; mv $TEST_FOLDER/test_results.xcresult $TEST_FOLDER/test_results_attempt1.xcresult 2>/dev/null; for f in $TEST_FOLDER/test_results*; do [ "$f" = "$TEST_FOLDER/test_results_attempt1.xcresult" ] || rm -rf "$f"; done; xcrun simctl shutdown all
-          new_command_on_retry: xcodebuild -project v8ios.xcodeproj -scheme TestRunner -resultBundlePath $TEST_FOLDER/test_results -destination platform\=iOS\ Simulator,OS\=latest,name\=iPhone\ 16\ Pro build test
+          new_command_on_retry: xcodebuild -project v8ios.xcodeproj -scheme TestRunner -resultBundlePath $TEST_FOLDER/test_results -destination platform\=iOS\ Simulator,OS\=latest,name\=iPhone\ 16\ Pro ${{ matrix.xcodebuild-args }} build test
       - name: Extract test results
         # Runs even when the test step failed: the xcresult usually still
         # carries the JS suite's junit attachments for the report step below.
@@ -122,11 +136,12 @@ jobs:
         uses: ./.github/actions/collect-test-diagnostics
         with:
           test-folder: ${{env.TEST_FOLDER}}
+          artifact-name: test-diagnostics${{ matrix.artifact-suffix }}
       - name: Archive Test Result Data
         if: always()
         uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
         with:
-          name: test-results
+          name: test-results${{ matrix.artifact-suffix }}
           path: |
             ${{env.TEST_FOLDER}}/test_results.xcresult
             ${{env.TEST_FOLDER}}/test_results_attempt1.xcresult

TestFixtures/TNSTestCommon.h

@@ -4,6 +4,8 @@ extern "C" {
 
 bool TNSIsConfigurationDebug();
 
+bool TNSIsAddressSanitizerEnabled();
+
 NSString* TNSGetOutput();
 
 void TNSLog(NSString*);

TestFixtures/TNSTestCommon.m

@@ -11,6 +11,14 @@ bool TNSIsConfigurationDebug() {
 #endif
 }
 
+bool TNSIsAddressSanitizerEnabled() {
+#if __has_feature(address_sanitizer)
+  return true;
+#else
+  return false;
+#endif
+}
+
 NSString* TNSGetOutput() {
     if (TNSTestOutput == nil) {
         TNSTestOutput = [NSMutableString new];

TestFixtures/exported-symbols.txt

@@ -56,6 +56,7 @@ _functionWithUnichar
 _functionWithUShort
 _functionWithUShortPtr
 _TNSIsConfigurationDebug
+_TNSIsAddressSanitizerEnabled
 _TNSClearOutput
 _TNSConstant
 _TNSConstant10_0Plus

TestRunner/app/tests/WorkerOptionsTests.js

@@ -43,6 +43,19 @@ describe("Worker platform options", function () {
         };
     };
 
+    // In an AddressSanitizer build a worker thread asked for anything below
+    // user-initiated still reads back user-initiated, so those classes cannot
+    // be observed there.
+    var expectQos = function (options, expected, done) {
+        if (TNSIsAddressSanitizerEnabled() && expected < NSQualityOfService.UserInitiated) {
+            pending("quality of service below user-initiated is not observable under AddressSanitizer");
+            return;
+        }
+        reportQos(options, done, function (qos) {
+            expect(qos).toBe(expected);
+        });
+    };
+
     // Background is deliberately absent: the system defines that class as work
     // that may take minutes, and on a loaded host a background thread has not
     // finished booting an isolate within two minutes. It is covered below
@@ -56,9 +69,7 @@ describe("Worker platform options", function () {
 
     priorities.forEach(function (pair) {
         it("runs the worker thread at " + pair[0] + " quality of service", function (done) {
-            reportQos({ ios: { priority: pair[0] } }, done, function (qos) {
-                expect(qos).toBe(pair[1]);
-            });
+            expectQos({ ios: { priority: pair[0] } }, pair[1], done);
         });
     });
 
@@ -71,21 +82,16 @@ describe("Worker platform options", function () {
     });
 
     it("still honors the deprecated iosPriority option", function (done) {
-        reportQos({ iosPriority: "utility" }, done, function (qos) {
-            expect(qos).toBe(NSQualityOfService.Utility);
-        });
+        expectQos({ iosPriority: "utility" }, NSQualityOfService.Utility, done);
     });
 
     it("prefers ios.priority over iosPriority when both are given", function (done) {
-        reportQos({ ios: { priority: "userInteractive" }, iosPriority: "background" }, done, function (qos) {
-            expect(qos).toBe(NSQualityOfService.UserInteractive);
-        });
+        expectQos({ ios: { priority: "userInteractive" }, iosPriority: "background" },
+            NSQualityOfService.UserInteractive, done);
     });
 
     it("ignores unknown keys inside ios", function (done) {
-        reportQos({ ios: { priority: "utility", somethingElse: 42 } }, done, function (qos) {
-            expect(qos).toBe(NSQualityOfService.Utility);
-        });
+        expectQos({ ios: { priority: "utility", somethingElse: 42 } }, NSQualityOfService.Utility, done);
     });
 
     it("starts a worker given no options at all", function (done) {
@@ -95,9 +101,7 @@ describe("Worker platform options", function () {
     });
 
     it("treats ios: null like an absent ios", function (done) {
-        reportQos({ ios: null, iosPriority: "utility" }, done, function (qos) {
-            expect(qos).toBe(NSQualityOfService.Utility);
-        });
+        expectQos({ ios: null, iosPriority: "utility" }, NSQualityOfService.Utility, done);
     });
 
     it("propagates the error thrown by an option getter", function () {

TestRunnerTests/ModuleTestServer.swift

@@ -22,21 +22,57 @@ final class ModuleTestServer {
     private let handler: Handler
     private var connections: [ObjectIdentifier: NWConnection] = [:]
 
-    init(port: UInt16, handler: @escaping Handler) throws {
+    enum StartError: Error {
+        case timedOut
+        case noPort
+    }
+
+    /// The loopback port the system assigned; valid once `start()` returned.
+    private(set) var port: UInt16 = 0
+
+    /// The port is left to the system rather than fixed: simulators share the
+    /// host's loopback interface, so two test runs on one machine would
+    /// otherwise answer each other's requests.
+    init(handler: @escaping Handler) throws {
         self.handler = handler
         let params = NWParameters.tcp
-        params.allowLocalEndpointReuse = true
         params.requiredLocalEndpoint = NWEndpoint.hostPort(
             host: NWEndpoint.Host("127.0.0.1"),
-            port: NWEndpoint.Port(rawValue: port)!)
+            port: .any)
         listener = try NWListener(using: params)
         listener.newConnectionHandler = { [weak self] connection in
             self?.accept(connection)
         }
     }
 
-    func start() {
+    /// Blocks until the listener is bound, because the port is only known then.
+    func start() throws {
+        let settled = DispatchSemaphore(value: 0)
+        var failure: Error?
+        listener.stateUpdateHandler = { state in
+            switch state {
+            case .ready:
+                settled.signal()
+            case .failed(let error):
+                failure = error
+                settled.signal()
+            default:
+                break
+            }
+        }
         listener.start(queue: queue)
+        let outcome = settled.wait(timeout: .now() + 10)
+        listener.stateUpdateHandler = nil
+        if outcome == .timedOut {
+            throw StartError.timedOut
+        }
+        if let failure = failure {
+            throw failure
+        }
+        guard let bound = listener.port?.rawValue, bound != 0 else {
+            throw StartError.noPort
+        }
+        port = bound
     }
 
     func stop() {

TestRunnerTests/TestRunnerTests.swift

@@ -1,7 +1,6 @@
 import XCTest
 
 class TestRunnerTests: XCTestCase {
-    private let port = 63846
     private var server: ModuleTestServer!
     private var runtimeUnitTestsExpectation: XCTestExpectation!
     private var reportDeliveryFailureReason: String?
@@ -12,15 +11,15 @@ class TestRunnerTests: XCTestCase {
     private let progressLock = NSLock()
     private var lastSpecSeen = "(no spec reported yet)"
 
-    override func setUp() {
+    override func setUpWithError() throws {
         continueAfterFailure = false
 
         // Standalone (not via self.expectation(...)) so we can drive it through
         // XCTWaiter alongside the crash watchdog without tripping the
         // XCTestCase "must waitForExpectations" rule.
         runtimeUnitTestsExpectation = XCTestExpectation(description: "Jasmine tests")
 
-        self.server = try! ModuleTestServer(port: UInt16(port)) {
+        self.server = try ModuleTestServer {
             (
                 environ: [String: Any],
                 startResponse: @escaping ((String, [(String, String)]) -> Void),
@@ -193,11 +192,12 @@ class TestRunnerTests: XCTestCase {
             sendBody(Data("Not Found".utf8))
         }
 
-        server.start()
+        try server.start()
+        print("ModuleTestServer listening on 127.0.0.1:\(server.port)")
     }
 
     override func tearDown() {
-        server.stop()
+        server?.stop()
     }
 
     func testRuntime() {
@@ -210,7 +210,7 @@ class TestRunnerTests: XCTestCase {
         let jasmineTestsTimeout: TimeInterval = 600
 
         let app = XCUIApplication()
-        app.launchEnvironment["REPORT_BASEURL"] = "http://127.0.0.1:\(port)/junit_report"
+        app.launchEnvironment["REPORT_BASEURL"] = "http://127.0.0.1:\(server.port)/junit_report"
         // The app's report retries and delivery_failed sentinel count from its
         // launch, which precedes the wait below — keep a margin so delivery
         // gives up (and the sentinel lands) before our timeout fires.