From 9a7075f46418b305f7556eb3101312d7b28f5449 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Mon, 5 Oct 2026 14:11:47 -0300 Subject: [PATCH 1/4] 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. --- .../collect-test-diagnostics/action.yml | 7 +++- .github/workflows/pull_request.yml | 23 ++++++++++--- TestFixtures/TNSTestCommon.h | 2 ++ TestFixtures/TNSTestCommon.m | 8 +++++ TestFixtures/exported-symbols.txt | 1 + TestRunner/app/tests/WorkerOptionsTests.js | 34 +++++++++++-------- 6 files changed, 55 insertions(+), 20 deletions(-) diff --git a/.github/actions/collect-test-diagnostics/action.yml b/.github/actions/collect-test-diagnostics/action.yml index 08ce858b0..49603765c 100644 --- a/.github/actions/collect-test-diagnostics/action.yml +++ b/.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 diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 41999f7da..857c93fd1 100644 --- a/.github/workflows/pull_request.yml +++ b/.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 diff --git a/TestFixtures/TNSTestCommon.h b/TestFixtures/TNSTestCommon.h index ab10e56f5..f68b47561 100644 --- a/TestFixtures/TNSTestCommon.h +++ b/TestFixtures/TNSTestCommon.h @@ -4,6 +4,8 @@ extern "C" { bool TNSIsConfigurationDebug(); +bool TNSIsAddressSanitizerEnabled(); + NSString* TNSGetOutput(); void TNSLog(NSString*); diff --git a/TestFixtures/TNSTestCommon.m b/TestFixtures/TNSTestCommon.m index 446b3b10a..7fa68ede0 100644 --- a/TestFixtures/TNSTestCommon.m +++ b/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]; diff --git a/TestFixtures/exported-symbols.txt b/TestFixtures/exported-symbols.txt index fb47d646c..9a915ec8e 100644 --- a/TestFixtures/exported-symbols.txt +++ b/TestFixtures/exported-symbols.txt @@ -56,6 +56,7 @@ _functionWithUnichar _functionWithUShort _functionWithUShortPtr _TNSIsConfigurationDebug +_TNSIsAddressSanitizerEnabled _TNSClearOutput _TNSConstant _TNSConstant10_0Plus diff --git a/TestRunner/app/tests/WorkerOptionsTests.js b/TestRunner/app/tests/WorkerOptionsTests.js index 3848d249a..12b454595 100644 --- a/TestRunner/app/tests/WorkerOptionsTests.js +++ b/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 () { From 3d19f243b4126235bb109b21efeadca19d5c42f4 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Mon, 5 Oct 2026 14:18:09 -0300 Subject: [PATCH 2/4] 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. --- TestRunnerTests/ModuleTestServer.swift | 44 +++++++++++++++++++++++--- TestRunnerTests/TestRunnerTests.swift | 7 ++-- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/TestRunnerTests/ModuleTestServer.swift b/TestRunnerTests/ModuleTestServer.swift index 1cea97be4..12950f19a 100644 --- a/TestRunnerTests/ModuleTestServer.swift +++ b/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() { diff --git a/TestRunnerTests/TestRunnerTests.swift b/TestRunnerTests/TestRunnerTests.swift index 8fb89a465..0d7356b6b 100644 --- a/TestRunnerTests/TestRunnerTests.swift +++ b/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? @@ -20,7 +19,7 @@ class TestRunnerTests: XCTestCase { // 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,7 +192,7 @@ class TestRunnerTests: XCTestCase { sendBody(Data("Not Found".utf8)) } - server.start() + try! server.start() } override func tearDown() { @@ -210,7 +209,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. From 6de71369f1f37089bd4b4d4533b5ce3e75fa1206 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Mon, 5 Oct 2026 14:22:33 -0300 Subject: [PATCH 3/4] 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. --- TestRunnerTests/TestRunnerTests.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/TestRunnerTests/TestRunnerTests.swift b/TestRunnerTests/TestRunnerTests.swift index 0d7356b6b..d901f739f 100644 --- a/TestRunnerTests/TestRunnerTests.swift +++ b/TestRunnerTests/TestRunnerTests.swift @@ -193,6 +193,7 @@ class TestRunnerTests: XCTestCase { } try! server.start() + print("ModuleTestServer listening on 127.0.0.1:\(server.port)") } override func tearDown() { From 84aa8b3c692aff6c5c366400efcce59ca990a71a Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Mon, 5 Oct 2026 14:51:19 -0300 Subject: [PATCH 4/4] 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. --- TestRunnerTests/TestRunnerTests.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/TestRunnerTests/TestRunnerTests.swift b/TestRunnerTests/TestRunnerTests.swift index d901f739f..6da9dc1fe 100644 --- a/TestRunnerTests/TestRunnerTests.swift +++ b/TestRunnerTests/TestRunnerTests.swift @@ -11,7 +11,7 @@ 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 @@ -19,7 +19,7 @@ class TestRunnerTests: XCTestCase { // XCTestCase "must waitForExpectations" rule. runtimeUnitTestsExpectation = XCTestExpectation(description: "Jasmine tests") - self.server = try! ModuleTestServer { + self.server = try ModuleTestServer { ( environ: [String: Any], startResponse: @escaping ((String, [(String, String)]) -> Void), @@ -192,12 +192,12 @@ class TestRunnerTests: XCTestCase { sendBody(Data("Not Found".utf8)) } - try! server.start() + try server.start() print("ModuleTestServer listening on 127.0.0.1:\(server.port)") } override func tearDown() { - server.stop() + server?.stop() } func testRuntime() {