From e5acb00a9fc8f02b433ca27981f752160e2fbfca Mon Sep 17 00:00:00 2001 From: gayanW Date: Wed, 19 Nov 2025 11:06:09 +0900 Subject: [PATCH 1/8] Add test test_jasmine.py#test_record_tests_without_filename --- .../jasmine/jasmine-test-results-v3.99.0.json | 150 ++++++++++++++++++ tests/test_runners/test_jasmine.py | 15 +- 2 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 tests/data/jasmine/jasmine-test-results-v3.99.0.json diff --git a/tests/data/jasmine/jasmine-test-results-v3.99.0.json b/tests/data/jasmine/jasmine-test-results-v3.99.0.json new file mode 100644 index 000000000..af251ead3 --- /dev/null +++ b/tests/data/jasmine/jasmine-test-results-v3.99.0.json @@ -0,0 +1,150 @@ +{ + "suite2": { + "id": "suite2", + "description": "when song has been paused", + "fullName": "Player when song has been paused", + "failedExpectations": [], + "deprecationWarnings": [], + "duration": 2, + "properties": null, + "status": "passed", + "specs": [ + { + "id": "spec0", + "description": "should be able to play a Song", + "fullName": "Player should be able to play a Song", + "failedExpectations": [], + "passedExpectations": [ + { + "matcherName": "toEqual", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toBePlaying", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "deprecationWarnings": [], + "pendingReason": "", + "duration": 0, + "properties": null, + "status": "passed" + }, + { + "id": "spec2", + "description": "should be possible to resume", + "fullName": "Player when song has been paused should be possible to resume", + "failedExpectations": [], + "passedExpectations": [ + { + "matcherName": "toBeTruthy", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toEqual", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "deprecationWarnings": [], + "pendingReason": "", + "duration": 1, + "properties": null, + "status": "passed" + }, + { + "id": "spec1", + "description": "should indicate that the song is currently paused", + "fullName": "Player when song has been paused should indicate that the song is currently paused", + "failedExpectations": [], + "passedExpectations": [ + { + "matcherName": "toBeFalsy", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toBePlaying", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "deprecationWarnings": [], + "pendingReason": "", + "duration": 1, + "properties": null, + "status": "passed" + } + ] + }, + "suite3": { + "id": "suite3", + "description": "#resume", + "fullName": "Player #resume", + "failedExpectations": [], + "deprecationWarnings": [], + "duration": 1, + "properties": null, + "status": "passed", + "specs": [ + { + "id": "spec3", + "description": "tells the current song if the user has made it a favorite", + "fullName": "Player tells the current song if the user has made it a favorite", + "failedExpectations": [], + "passedExpectations": [ + { + "matcherName": "toHaveBeenCalledWith", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "deprecationWarnings": [], + "pendingReason": "", + "duration": 1, + "properties": null, + "status": "passed" + }, + { + "id": "spec4", + "description": "should throw an exception if song is already playing", + "fullName": "Player #resume should throw an exception if song is already playing", + "failedExpectations": [], + "passedExpectations": [ + { + "matcherName": "toThrowError", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "deprecationWarnings": [], + "pendingReason": "", + "duration": 0, + "properties": null, + "status": "passed" + } + ] + }, + "suite1": { + "id": "suite1", + "description": "Player", + "fullName": "Player", + "failedExpectations": [], + "deprecationWarnings": [], + "duration": 4, + "properties": null, + "status": "passed", + "specs": [] + } +} diff --git a/tests/test_runners/test_jasmine.py b/tests/test_runners/test_jasmine.py index 2a49d07ad..689119292 100644 --- a/tests/test_runners/test_jasmine.py +++ b/tests/test_runners/test_jasmine.py @@ -11,13 +11,26 @@ class JasmineTest(CliTestCase): @responses.activate @mock.patch.dict(os.environ, {"LAUNCHABLE_TOKEN": CliTestCase.launchable_token}) - def test_record_test_json(self): + def test_record_tests_json(self): result = self.cli('record', 'tests', '--session', self.session, 'jasmine', str(self.test_files_dir.joinpath("jasmine-test-results.json"))) self.assert_success(result) self.assert_record_tests_payload('record_test_result.json') + @responses.activate + @mock.patch.dict(os.environ, + {"LAUNCHABLE_TOKEN": CliTestCase.launchable_token}) + def test_record_tests_without_filename(self): + result = self.cli('record', 'tests', '--session', self.session, + 'jasmine', str(self.test_files_dir.joinpath("jasmine-test-results-v3.99.0.json"))) + + self.assertIn( + "does not appear to be valid format. " + "Make sure you are using Jasmine >= v4.6.0 and jasmine-json-test-reporter as the reporter.", + result.output + ) + @responses.activate @mock.patch.dict(os.environ, {"LAUNCHABLE_TOKEN": CliTestCase.launchable_token}) From 2f7f23de613a709cf70689a9de5b1acb82f5fbaa Mon Sep 17 00:00:00 2001 From: gayanW Date: Wed, 19 Nov 2025 11:06:45 +0900 Subject: [PATCH 2/8] Add tests/data/jasmine/README.md --- tests/data/jasmine/README.md | 41 ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 tests/data/jasmine/README.md diff --git a/tests/data/jasmine/README.md b/tests/data/jasmine/README.md new file mode 100644 index 000000000..a7e67a966 --- /dev/null +++ b/tests/data/jasmine/README.md @@ -0,0 +1,41 @@ +Jasmine Runner Usage +===================== + +If you want to test this runner from scratch, start by creating a new jasmine project: + +``` +% npm install --save-dev jasmine jasmine-json-test-reporter +% npx jasmine init +% npx jasmine example +% git add . && git commit -m "Initial commit" +``` + +Create spec/helpers/jasmine-json-test-reporter.js +``` +var JSONReporter = require('jasmine-json-test-reporter'); +jasmine.getEnv().addReporter(new JSONReporter({ + file: 'jasmine-report.json' +})); +``` + +Record tests +``` +BUILD_NAME=jasmine_build +launchable record build --name ${BUILD_NAME} +launchable record session --build ${BUILD_NAME} > session.txt + +# Write all tests to a file +find spec/jasmine_examples -type f > test_list.txt + +# Run all tests +npx jasmine $(cat test_list.txt) + +launchable record tests --base $(pwd) jasmine jasmine-report.json +``` + +Request subset +``` +cat test_list.txt | launchable subset --target 25% jasmine > subset.txt +npx jasmine $(cat subset.txt) +``` + From 28185c47f4387a856e735f7f22e680fd82af585f Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 20 Nov 2025 13:20:20 -0800 Subject: [PATCH 3/8] [chore] better disambiguation with Commit GraphCollector I always get confused which is which. Naming this `Main` makes it really clear. --- .../commits/{CommitIngester.java => Main.java} | 6 +++--- .../com/launchableinc/ingest/commits/AllTests.java | 2 +- .../{CommitIngesterTest.java => MainTest.java} | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) rename src/main/java/com/launchableinc/ingest/commits/{CommitIngester.java => Main.java} (97%) rename src/test/java/com/launchableinc/ingest/commits/{CommitIngesterTest.java => MainTest.java} (91%) diff --git a/src/main/java/com/launchableinc/ingest/commits/CommitIngester.java b/src/main/java/com/launchableinc/ingest/commits/Main.java similarity index 97% rename from src/main/java/com/launchableinc/ingest/commits/CommitIngester.java rename to src/main/java/com/launchableinc/ingest/commits/Main.java index 2efff19c0..463e2cac4 100644 --- a/src/main/java/com/launchableinc/ingest/commits/CommitIngester.java +++ b/src/main/java/com/launchableinc/ingest/commits/Main.java @@ -17,7 +17,7 @@ import org.kohsuke.args4j.Option; /** Driver for {@link CommitGraphCollector}. */ -public class CommitIngester { +public class Main { @Argument(required = true, metaVar = "NAME", usage = "Uniquely identifies this repository within the workspace", index = 0) public String name; @@ -60,10 +60,10 @@ public class CommitIngester { @VisibleForTesting String launchableToken = null; - public CommitIngester() throws CmdLineException, MalformedURLException {} + public Main() throws CmdLineException, MalformedURLException {} public static void main(String[] args) throws Exception { - CommitIngester ingester = new CommitIngester(); + Main ingester = new Main(); CmdLineParser parser = new CmdLineParser(ingester); try { parser.parseArgument(args); diff --git a/src/test/java/com/launchableinc/ingest/commits/AllTests.java b/src/test/java/com/launchableinc/ingest/commits/AllTests.java index ab9b02858..dc1315cab 100644 --- a/src/test/java/com/launchableinc/ingest/commits/AllTests.java +++ b/src/test/java/com/launchableinc/ingest/commits/AllTests.java @@ -7,7 +7,7 @@ @RunWith(Suite.class) @SuiteClasses({ CommitGraphCollectorTest.class, - CommitIngesterTest.class, + MainTest.class, FileChunkStreamerTest.class, SSLBypassTest.class, ProgressReportingConsumerTest.class diff --git a/src/test/java/com/launchableinc/ingest/commits/CommitIngesterTest.java b/src/test/java/com/launchableinc/ingest/commits/MainTest.java similarity index 91% rename from src/test/java/com/launchableinc/ingest/commits/CommitIngesterTest.java rename to src/test/java/com/launchableinc/ingest/commits/MainTest.java index fccd4fe2f..b0702bdb1 100644 --- a/src/test/java/com/launchableinc/ingest/commits/CommitIngesterTest.java +++ b/src/test/java/com/launchableinc/ingest/commits/MainTest.java @@ -19,7 +19,7 @@ import org.mockserver.junit.MockServerRule; @RunWith(JUnit4.class) -public class CommitIngesterTest { +public class MainTest { @Rule public TemporaryFolder tmp = new TemporaryFolder(); @Rule public MockServerRule mockServerRule = new MockServerRule(this); private MockServerClient mockServerClient; @@ -63,14 +63,14 @@ public void specifySubmodule() throws Exception { return response().withBody("OK"); }); - CommitIngester commitIngester = new CommitIngester(); + Main main = new Main(); InetSocketAddress addr = mockServerClient.remoteAddress(); // Specify submodule as the repository. JGit cannot open this directly, so the code should open // the main repository first, then open the submodule. - commitIngester.repo = new File(mainrepoDir, "sub"); - commitIngester.url = + main.repo = new File(mainrepoDir, "sub"); + main.url = new URL(String.format("http://%s:%s/intake/", addr.getHostString(), addr.getPort())); - commitIngester.launchableToken = "v1:testorg/testws:dummy-token"; - commitIngester.run(); + main.launchableToken = "v1:testorg/testws:dummy-token"; + main.run(); } } From 79364c916b9649f3a4dfcf1480025eaf72ac6e20 Mon Sep 17 00:00:00 2001 From: gayanW Date: Fri, 21 Nov 2025 17:04:11 +0900 Subject: [PATCH 4/8] Add karma, and ng test runners --- launchable/test_runners/karma.py | 153 ++++++++++ launchable/test_runners/ng.py | 22 ++ tests/data/karma/record_test_result.json | 189 ++++++++++++ tests/data/karma/sample-report.json | 360 +++++++++++++++++++++++ tests/data/ng/subset_payload.json | 16 + tests/test_runners/test_karma.py | 18 ++ tests/test_runners/test_ng.py | 22 ++ 7 files changed, 780 insertions(+) create mode 100644 launchable/test_runners/karma.py create mode 100644 launchable/test_runners/ng.py create mode 100644 tests/data/karma/record_test_result.json create mode 100644 tests/data/karma/sample-report.json create mode 100644 tests/data/ng/subset_payload.json create mode 100644 tests/test_runners/test_karma.py create mode 100644 tests/test_runners/test_ng.py diff --git a/launchable/test_runners/karma.py b/launchable/test_runners/karma.py new file mode 100644 index 000000000..ffd471208 --- /dev/null +++ b/launchable/test_runners/karma.py @@ -0,0 +1,153 @@ +# This runner only supports recording tests +# For subsetting, use 'ng' test runner instead +# It's possible to use 'karma' runner for recording, and 'ng' runner for subsetting, for the same test session +import json +from typing import Dict, Generator, List + +import click + +from ..commands.record.case_event import CaseEvent +from ..testpath import TestPath +from . import launchable + + +@click.argument('reports', required=True, nargs=-1) +@launchable.record.tests +def record_tests(client, reports): + client.parse_func = JSONReportParser(client).parse_func + + for r in reports: + client.report(r) + + client.run() + + +class JSONReportParser: + """ + Sample Karma report format: + { + "browsers": {...}, + "result": { + "24461741": [ + { + "fullName": "path/to/spec.ts should do something", + "description": "should do something", + "id": "spec0", + "log": [], + "skipped": false, + "disabled": false, + "pending": false, + "success": true, + "suite": [ + "path/to/spec.ts" + ], + "time": 92, + "executedExpectationsCount": 1, + "passedExpectations": [...], + "properties": null + } + ] + }, + "summary": {...} + } + """ + + def __init__(self, client): + self.client = client + + def parse_func(self, report_file: str) -> Generator[Dict, None, None]: # type: ignore + data: Dict + with open(report_file, 'r') as json_file: + try: + data = json.load(json_file) + except Exception: + click.echo( + click.style("Error: Failed to load Json report file: {}".format(report_file), fg='red'), err=True) + return + + if not self._validate_report_format(data): + click.echo( + "Error: {} does not appear to be valid Karma report format. " + "Make sure you are using karma-json-reporter or a compatible reporter.".format( + report_file), err=True) + return + + results = data.get("result", {}) + for browser_id, specs in results.items(): + if isinstance(specs, list): + for event in self._parse_specs(specs): + yield event + + def _validate_report_format(self, data: Dict) -> bool: + if not isinstance(data, dict): + return False + + if "result" not in data: + return False + + results = data.get("result", {}) + if not isinstance(results, dict): + return False + + for browser_id, specs in results.items(): + if not isinstance(specs, list): + return False + + for spec in specs: + if not isinstance(spec, dict): + return False + # Check for required fields + if "suite" not in spec or "time" not in spec: + return False + # Field suite should have at least one element (filename) + suite = spec.get("suite", []) + if not isinstance(suite, list) or len(suite) == 0: + return False + + return True + + def _parse_specs(self, specs: List[Dict]) -> List[Dict]: + events: List[Dict] = [] + + for spec in specs: + # TODO: + # In NextWorld, test filepaths are included in the suite tag + # But generally in a Karma test report, a suite tag can be any string + # For the time being let's get filepaths from the suite tag, + # until we find a standard way to include filepaths in the test reports + suite = spec.get("suite", []) + filename = suite[0] if suite else "" + + test_path: TestPath = [ + self.client.make_file_path_component(filename), + {"type": "testcase", "name": spec.get("fullName", spec.get("description", ""))} + ] + + duration_msec = spec.get("time", 0) + status = self._case_event_status_from_spec(spec) + stderr = self._parse_stderr(spec) + + events.append(CaseEvent.create( + test_path=test_path, + duration_secs=duration_msec / 1000 if duration_msec else 0, + status=status, + stderr=stderr + )) + + return events + + def _case_event_status_from_spec(self, spec: Dict) -> int: + if spec.get("skipped", False) or spec.get("disabled", False) or spec.get("pending", False): + return CaseEvent.TEST_SKIPPED + + if spec.get("success", False): + return CaseEvent.TEST_PASSED + else: + return CaseEvent.TEST_FAILED + + def _parse_stderr(self, spec: Dict) -> str: + log_messages = spec.get("log", []) + if not log_messages: + return "" + + return "\n".join(str(msg) for msg in log_messages if msg) diff --git a/launchable/test_runners/ng.py b/launchable/test_runners/ng.py new file mode 100644 index 000000000..09b4cc509 --- /dev/null +++ b/launchable/test_runners/ng.py @@ -0,0 +1,22 @@ +from . import launchable + + +@launchable.subset +def subset(client): + """ + Input format example: + src/app/feature/feature.component.spec.ts + src/app/service/service.service.spec.ts + + Output format: --include= format that can be passed to ng test + Example: + --include=src/app/feature/feature.component.spec.ts --include=src/app/service/service.service.spec.ts + """ + for t in client.stdin(): + path = t.strip() + if path: + client.test_path(path) + + client.formatter = lambda x: "--include={}".format(x[0]['name']) + client.separator = " " + client.run() diff --git a/tests/data/karma/record_test_result.json b/tests/data/karma/record_test_result.json new file mode 100644 index 000000000..d0972d26d --- /dev/null +++ b/tests/data/karma/record_test_result.json @@ -0,0 +1,189 @@ +{ + "events": [ + { + "type": "case", + "testPath": [ + { + "type": "file", + "name": "foo/bar/zot.spec.ts" + }, + { + "type": "testcase", + "name": "foo/bar/zot.spec.ts should feed the monkey" + } + ], + "duration": 0.092, + "status": 1, + "stdout": "", + "stderr": "", + "data": null + }, + { + "type": "case", + "testPath": [ + { + "type": "file", + "name": "foo/bar/zot.spec.ts" + }, + { + "type": "testcase", + "name": "foo/bar/zot.spec.ts should fetch the toy" + } + ], + "duration": 0.027, + "status": 1, + "stdout": "", + "stderr": "", + "data": null + }, + { + "type": "case", + "testPath": [ + { + "type": "file", + "name": "foo/bar/zot.spec.ts" + }, + { + "type": "testcase", + "name": "foo/bar/zot.spec.ts should fetch the toyForRecord - record found" + } + ], + "duration": 0.028, + "status": 1, + "stdout": "", + "stderr": "", + "data": null + }, + { + "type": "case", + "testPath": [ + { + "type": "file", + "name": "foo/bar/zot.spec.ts" + }, + { + "type": "testcase", + "name": "foo/bar/zot.spec.ts should fetch the toyForRecord - record not found" + } + ], + "duration": 0.027, + "status": 1, + "stdout": "", + "stderr": "", + "data": null + }, + { + "type": "case", + "testPath": [ + { + "type": "file", + "name": "foo/bar/zot.spec.ts" + }, + { + "type": "testcase", + "name": "foo/bar/zot.spec.ts should fetch the toyForRecord - error in response" + } + ], + "duration": 0.033, + "status": 1, + "stdout": "", + "stderr": "", + "data": null + }, + { + "type": "case", + "testPath": [ + { + "type": "file", + "name": "foo/bar/zot.spec.ts" + }, + { + "type": "testcase", + "name": "foo/bar/zot.spec.ts should throw a ball" + } + ], + "duration": 0.026, + "status": 1, + "stdout": "", + "stderr": "", + "data": null + }, + { + "type": "case", + "testPath": [ + { + "type": "file", + "name": "foo/bar/zot.spec.ts" + }, + { + "type": "testcase", + "name": "foo/bar/zot.spec.ts should be nice to the zookeeper" + } + ], + "duration": 0.024, + "status": 1, + "stdout": "", + "stderr": "", + "data": null + }, + { + "type": "case", + "testPath": [ + { + "type": "file", + "name": "foo/bar/zot.spec.ts" + }, + { + "type": "testcase", + "name": "foo/bar/zot.spec.ts should like a banana or two" + } + ], + "duration": 0.024, + "status": 1, + "stdout": "", + "stderr": "", + "data": null + }, + { + "type": "case", + "testPath": [ + { + "type": "file", + "name": "foo/bar/zot.spec.ts" + }, + { + "type": "testcase", + "name": "foo/bar/zot.spec.ts should fall back to an apple if banana is not available" + } + ], + "duration": 0.025, + "status": 1, + "stdout": "", + "stderr": "", + "data": null + }, + { + "type": "case", + "testPath": [ + { + "type": "file", + "name": "foo/bar/zot.spec.ts" + }, + { + "type": "testcase", + "name": "foo/bar/zot.spec.ts should accept oranges if apple is not available" + } + ], + "duration": 0.032, + "status": 1, + "stdout": "", + "stderr": "", + "data": null + } + ], + "testRunner": "karma", + "group": "", + "noBuild": false, + "flavors": [], + "testSuite": "" +} diff --git a/tests/data/karma/sample-report.json b/tests/data/karma/sample-report.json new file mode 100644 index 000000000..ada7cea12 --- /dev/null +++ b/tests/data/karma/sample-report.json @@ -0,0 +1,360 @@ +{ + "browsers": { + "123456": { + "id": "123456", + "fullName": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/131.0.0.0 Safari/537.36", + "name": "Chrome Headless 131.0.0.0 (Mac OS 10.15.7)", + "state": "DISCONNECTED", + "lastResult": { + "startTime": 1763588893304, + "total": 10, + "success": 10, + "failed": 0, + "skipped": 0, + "totalTime": 358, + "netTime": 338, + "error": true, + "disconnected": false + }, + "disconnectsCount": 0, + "noActivityTimeout": 30000, + "disconnectDelay": 2000 + } + }, + "result": { + "123456": [ + { + "fullName": "foo/bar/zot.spec.ts should feed the monkey", + "description": "should feed the monkey", + "id": "spec0", + "log": [], + "skipped": false, + "disabled": false, + "pending": false, + "success": true, + "suite": [ + "foo/bar/zot.spec.ts" + ], + "time": 92, + "executedExpectationsCount": 1, + "passedExpectations": [ + { + "matcherName": "toBeTruthy", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "properties": null + }, + { + "fullName": "foo/bar/zot.spec.ts should fetch the toy", + "description": "should fetch the toy", + "id": "spec1", + "log": [], + "skipped": false, + "disabled": false, + "pending": false, + "success": true, + "suite": [ + "foo/bar/zot.spec.ts" + ], + "time": 27, + "executedExpectationsCount": 7, + "passedExpectations": [ + { + "matcherName": "toHaveBeenCalled", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toHaveBeenCalled", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toHaveBeenCalled", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toHaveBeenCalled", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toHaveBeenCalled", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toHaveBeenCalledTimes", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toEqual", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "properties": null + }, + { + "fullName": "foo/bar/zot.spec.ts should fetch the toyForRecord - record found", + "description": "should fetch the toyForRecord - record found", + "id": "spec2", + "log": [], + "skipped": false, + "disabled": false, + "pending": false, + "success": true, + "suite": [ + "foo/bar/zot.spec.ts" + ], + "time": 28, + "executedExpectationsCount": 2, + "passedExpectations": [ + { + "matcherName": "toHaveBeenCalledOnceWith", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toEqual", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "properties": null + }, + { + "fullName": "foo/bar/zot.spec.ts should fetch the toyForRecord - record not found", + "description": "should fetch the toyForRecord - record not found", + "id": "spec3", + "log": [], + "skipped": false, + "disabled": false, + "pending": false, + "success": true, + "suite": [ + "foo/bar/zot.spec.ts" + ], + "time": 27, + "executedExpectationsCount": 2, + "passedExpectations": [ + { + "matcherName": "toHaveBeenCalledOnceWith", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toEqual", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "properties": null + }, + { + "fullName": "foo/bar/zot.spec.ts should fetch the toyForRecord - error in response", + "description": "should fetch the toyForRecord - error in response", + "id": "spec4", + "log": [], + "skipped": false, + "disabled": false, + "pending": false, + "success": true, + "suite": [ + "foo/bar/zot.spec.ts" + ], + "time": 33, + "executedExpectationsCount": 2, + "passedExpectations": [ + { + "matcherName": "toHaveBeenCalledOnceWith", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toEqual", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "properties": null + }, + { + "fullName": "foo/bar/zot.spec.ts should throw a ball", + "description": "should throw a ball", + "id": "spec5", + "log": [], + "skipped": false, + "disabled": false, + "pending": false, + "success": true, + "suite": [ + "foo/bar/zot.spec.ts" + ], + "time": 26, + "executedExpectationsCount": 2, + "passedExpectations": [ + { + "matcherName": "toEqual", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toHaveBeenCalledOnceWith", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "properties": null + }, + { + "fullName": "foo/bar/zot.spec.ts should be nice to the zookeeper", + "description": "should be nice to the zookeeper", + "id": "spec6", + "log": [], + "skipped": false, + "disabled": false, + "pending": false, + "success": true, + "suite": [ + "foo/bar/zot.spec.ts" + ], + "time": 24, + "executedExpectationsCount": 2, + "passedExpectations": [ + { + "matcherName": "toEqual", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toHaveBeenCalledOnceWith", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "properties": null + }, + { + "fullName": "foo/bar/zot.spec.ts should like a banana or two", + "description": "should like a banana or two", + "id": "spec7", + "log": [], + "skipped": false, + "disabled": false, + "pending": false, + "success": true, + "suite": [ + "foo/bar/zot.spec.ts" + ], + "time": 24, + "executedExpectationsCount": 2, + "passedExpectations": [ + { + "matcherName": "toEqual", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toHaveBeenCalledOnceWith", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "properties": null + }, + { + "fullName": "foo/bar/zot.spec.ts should fall back to an apple if banana is not available", + "description": "should fall back to an apple if banana is not available", + "id": "spec8", + "log": [], + "skipped": false, + "disabled": false, + "pending": false, + "success": true, + "suite": [ + "foo/bar/zot.spec.ts" + ], + "time": 25, + "executedExpectationsCount": 2, + "passedExpectations": [ + { + "matcherName": "toBeNull", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toHaveBeenCalledOnceWith", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "properties": null + }, + { + "fullName": "foo/bar/zot.spec.ts should accept oranges if apple is not available", + "description": "should accept oranges if apple is not available", + "id": "spec9", + "log": [], + "skipped": false, + "disabled": false, + "pending": false, + "success": true, + "suite": [ + "foo/bar/zot.spec.ts" + ], + "time": 32, + "executedExpectationsCount": 2, + "passedExpectations": [ + { + "matcherName": "toEqual", + "message": "Passed.", + "stack": "", + "passed": true + }, + { + "matcherName": "toHaveBeenCalledOnceWith", + "message": "Passed.", + "stack": "", + "passed": true + } + ], + "properties": null + } + ] + }, + "summary": { + "success": 10, + "failed": 0, + "skipped": 0, + "error": true, + "disconnected": false, + "exitCode": 1 + } +} diff --git a/tests/data/ng/subset_payload.json b/tests/data/ng/subset_payload.json new file mode 100644 index 000000000..d0b15b816 --- /dev/null +++ b/tests/data/ng/subset_payload.json @@ -0,0 +1,16 @@ +{ + "testPaths": [ + [ + { "type": "file", "name": "foo/bar/zot.spec.ts" } + ], + [ + { "type": "file", "name": "client-source/src/app/shared/other-test.spec.ts" } + ] + ], + "testRunner": "ng", + "goal": {"type": "subset-by-percentage", "percentage": 0.1}, + "ignoreNewTests": false, + "session": { "id": "16" }, + "getTestsFromGuess": false, + "getTestsFromPreviousSessions": false +} diff --git a/tests/test_runners/test_karma.py b/tests/test_runners/test_karma.py new file mode 100644 index 000000000..907904788 --- /dev/null +++ b/tests/test_runners/test_karma.py @@ -0,0 +1,18 @@ +import os +from unittest import mock + +import responses # type: ignore + +from tests.cli_test_case import CliTestCase + + +class KarmaTest(CliTestCase): + @responses.activate + @mock.patch.dict(os.environ, + {"LAUNCHABLE_TOKEN": CliTestCase.launchable_token}) + def test_record_tests_json(self): + result = self.cli('record', 'tests', '--session', self.session, + 'karma', str(self.test_files_dir.joinpath("sample-report.json"))) + + self.assert_success(result) + self.assert_record_tests_payload('record_test_result.json') diff --git a/tests/test_runners/test_ng.py b/tests/test_runners/test_ng.py new file mode 100644 index 000000000..36c090c29 --- /dev/null +++ b/tests/test_runners/test_ng.py @@ -0,0 +1,22 @@ +import os +from unittest import mock + +import responses # type: ignore + +from launchable.utils.session import write_build +from tests.cli_test_case import CliTestCase + + +class NgTest(CliTestCase): + @responses.activate + @mock.patch.dict(os.environ, + {"LAUNCHABLE_TOKEN": CliTestCase.launchable_token}) + def test_subset(self): + write_build(self.build_name) + + subset_input = """foo/bar/zot.spec.ts +client-source/src/app/shared/other-test.spec.ts +""" + result = self.cli('subset', '--target', '10%', 'ng', input=subset_input) + self.assert_success(result) + self.assert_subset_payload('subset_payload.json') From 76bfa3bb7cff849a943a26e9aff6f1287a9bb215 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Mon, 24 Nov 2025 11:43:15 -0800 Subject: [PATCH 5/8] [chore] error reporting improvement Tell apart a bad value vs no value. Also, use the ensure method to avoid duplicating errors in multiple places. Seeing incorrect value in LAUNCHABLE_TOKEN previously was a silently recovered error, but I think this behavior is harmful. If the value is incorrectly set, we need to call an attention to that fact so that we can guide users to fix it. --- launchable/commands/verify.py | 17 +++++++---------- launchable/utils/authentication.py | 10 ++++++---- launchable/utils/launchable_client.py | 10 ++-------- tests/utils/test_authentication.py | 6 ------ 4 files changed, 15 insertions(+), 28 deletions(-) diff --git a/launchable/commands/verify.py b/launchable/commands/verify.py index 68e4596b9..a7d47eb7b 100644 --- a/launchable/commands/verify.py +++ b/launchable/commands/verify.py @@ -11,7 +11,7 @@ from launchable.utils.env_keys import TOKEN_KEY from launchable.utils.tracking import Tracking, TrackingClient -from ..utils.authentication import get_org_workspace +from ..utils.authentication import get_org_workspace, ensure_org_workspace from ..utils.click import emoji from ..utils.commands import Command from ..utils.http_client import DEFAULT_BASE_URL @@ -88,18 +88,15 @@ def verify(context: click.core.Context): click.echo("Java command: " + repr(java)) click.echo("launchable version: " + repr(version)) - if org is None or workspace is None: - msg = ( - "Could not identify Launchable organization/workspace. " - "Please confirm if you set LAUNCHABLE_TOKEN or LAUNCHABLE_ORGANIZATION and LAUNCHABLE_WORKSPACE " - "environment variables" - ) + # raise an error here after we print out the basic diagnostics if LAUNCHABLE_TOKEN is not set. + try: + ensure_org_workspace() + except click.UsageError as e: tracking_client.send_error_event( event_name=Tracking.ErrorEvent.INTERNAL_CLI_ERROR, - stack_trace=msg + stack_trace=e.message ) - raise click.UsageError( - click.style(msg, fg="red")) + raise e try: res = client.request("get", "verification") diff --git a/launchable/utils/authentication.py b/launchable/utils/authentication.py index a6f01abb0..aee82dcfd 100644 --- a/launchable/utils/authentication.py +++ b/launchable/utils/authentication.py @@ -8,6 +8,10 @@ def get_org_workspace(): + ''' + Returns (org,ws) tuple from LAUNCHABLE_TOKEN, or (None,None) if not found. + Use ensure_org_workspace() if this is supposed to be an error condition + ''' token = os.getenv(TOKEN_KEY) if token: try: @@ -15,7 +19,7 @@ def get_org_workspace(): org, workspace = user.split("/", 1) return org, workspace except ValueError: - return None, None + raise click.UsageError(click.style("Invalid value in LAUNCHABLE_TOKEN environment variable.", fg="red")) return os.getenv(ORGANIZATION_KEY), os.getenv(WORKSPACE_KEY) @@ -25,9 +29,7 @@ def ensure_org_workspace() -> Tuple[str, str]: if org is None or workspace is None: raise click.UsageError( click.style( - "Could not identify Launchable organization/workspace. " - "Please confirm if you set LAUNCHABLE_TOKEN or LAUNCHABLE_ORGANIZATION and " - "LAUNCHABLE_WORKSPACE environment variables", + "LAUNCHABLE_TOKEN environment variable not set. Nor the LAUNCHABLE_ORGANIZATION and LAUNCHABLE_WORKSPACE combo if you are using https://help.launchableinc.com/sending-data-to-launchable/using-the-launchable-cli/getting-started/migration-to-github-oidc-auth/)", # noqa: E501 fg="red")) return org, workspace diff --git a/launchable/utils/launchable_client.py b/launchable/utils/launchable_client.py index 5013171eb..f2bb28153 100644 --- a/launchable/utils/launchable_client.py +++ b/launchable/utils/launchable_client.py @@ -9,7 +9,7 @@ from launchable.utils.tracking import Tracking, TrackingClient # type: ignore from ..app import Application -from .authentication import get_org_workspace +from .authentication import get_org_workspace, ensure_org_workspace from .env_keys import REPORT_ERROR_KEY @@ -23,13 +23,7 @@ def __init__(self, tracking_client: Optional[TrackingClient] = None, base_url: s app=app ) self.tracking_client = tracking_client - self.organization, self.workspace = get_org_workspace() - if self.organization is None or self.workspace is None: - raise ValueError( - "Could not identify a Launchable organization/workspace. " - "Confirm that you set LAUNCHABLE_TOKEN " - "(or LAUNCHABLE_ORGANIZATION and LAUNCHABLE_WORKSPACE) environment variable(s)\n" - "See https://help.launchableinc.com/getting-started#setting-your-api-key") + self.organization, self.workspace = ensure_org_workspace() self._workspace_state_cache: Optional[Dict[str, Union[str, bool]]] = None def request( diff --git a/tests/utils/test_authentication.py b/tests/utils/test_authentication.py index 9106b0bc5..e44874347 100644 --- a/tests/utils/test_authentication.py +++ b/tests/utils/test_authentication.py @@ -11,12 +11,6 @@ def test_get_org_workspace_no_environment_variables(self): self.assertIsNone(org) self.assertIsNone(workspace) - @mock.patch.dict(os.environ, {"LAUNCHABLE_TOKEN": "invalid"}) - def test_get_org_workspace_invalid_LAUNCHABLE_TOKEN(self): - org, workspace = get_org_workspace() - self.assertIsNone(org) - self.assertIsNone(workspace) - @mock.patch.dict(os.environ, {"LAUNCHABLE_TOKEN": "v1:launchableinc/test:token"}) def test_get_org_workspace_valid_LAUNCHABLE_TOKEN(self): From 5d6c939f7de106aadf27266b1548d8899970e7f1 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Tue, 25 Nov 2025 09:42:50 -0800 Subject: [PATCH 6/8] Adding a placeholder karma subset support --- launchable/test_runners/karma.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/launchable/test_runners/karma.py b/launchable/test_runners/karma.py index ffd471208..12ed811eb 100644 --- a/launchable/test_runners/karma.py +++ b/launchable/test_runners/karma.py @@ -11,6 +11,18 @@ from . import launchable +@click.option('--with', '_with') +@launchable.subset +def subset(client, _with: str): + # TODO: implement the --with ng option + + # read lines as test file names + for t in client.stdin(): + client.test_path(t.rstrip("\n")) + + client.run() + + @click.argument('reports', required=True, nargs=-1) @launchable.record.tests def record_tests(client, reports): From 3ba127e9419b77503a428e860bd4c6c6dd5b4a8a Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Tue, 25 Nov 2025 09:57:11 -0800 Subject: [PATCH 7/8] Added a test --- tests/data/karma/subset_result.json | 22 ++++++++++++++++++++++ tests/test_runners/test_karma.py | 12 ++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 tests/data/karma/subset_result.json diff --git a/tests/data/karma/subset_result.json b/tests/data/karma/subset_result.json new file mode 100644 index 000000000..d5642af31 --- /dev/null +++ b/tests/data/karma/subset_result.json @@ -0,0 +1,22 @@ +{ + "testPaths": [ + [ + { + "type": "file", + "name": "a.ts" + } + ], + [ + { + "type": "file", + "name": "b.ts" + } + ] + ], + "testRunner": "karma", + "session": { "id": "16" }, + "goal": {"type": "subset-by-percentage", "percentage": 0.1}, + "ignoreNewTests": false, + "getTestsFromGuess": false, + "getTestsFromPreviousSessions": false +} diff --git a/tests/test_runners/test_karma.py b/tests/test_runners/test_karma.py index 907904788..c8163cb86 100644 --- a/tests/test_runners/test_karma.py +++ b/tests/test_runners/test_karma.py @@ -3,6 +3,7 @@ import responses # type: ignore +from launchable.utils.session import write_build from tests.cli_test_case import CliTestCase @@ -16,3 +17,14 @@ def test_record_tests_json(self): self.assert_success(result) self.assert_record_tests_payload('record_test_result.json') + + @responses.activate + @mock.patch.dict(os.environ, {"LAUNCHABLE_TOKEN": CliTestCase.launchable_token}) + def test_subset(self): + # emulate launchable record build + write_build(self.build_name) + + result = self.cli('subset', '--target', '10%', '--base', + os.getcwd(), 'karma', '--with', 'ng', input="a.ts\nb.ts") + self.assert_success(result) + self.assert_subset_payload('subset_result.json') From 04162fc8eb6f18cd5390cc3a7af2f10a68791704 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Tue, 25 Nov 2025 10:50:36 -0800 Subject: [PATCH 8/8] Refactored common implementation --- launchable/test_runners/file.py | 9 +-------- launchable/test_runners/jasmine.py | 9 +-------- launchable/test_runners/launchable.py | 20 +++++++++++++++++++- launchable/test_runners/playwright.py | 9 +-------- launchable/test_runners/prove.py | 8 +------- launchable/test_runners/vitest.py | 8 +------- 6 files changed, 24 insertions(+), 39 deletions(-) diff --git a/launchable/test_runners/file.py b/launchable/test_runners/file.py index a9670d40a..daf725b4d 100644 --- a/launchable/test_runners/file.py +++ b/launchable/test_runners/file.py @@ -5,14 +5,7 @@ from . import launchable -@launchable.subset -def subset(client): - # read lines as test file names - for t in client.stdin(): - client.test_path(t.rstrip("\n")) - - client.run() - +subset = launchable.CommonSubsetImpls(__name__).scan_stdin() record_tests = launchable.CommonRecordTestImpls(__name__).file_profile_report_files() diff --git a/launchable/test_runners/jasmine.py b/launchable/test_runners/jasmine.py index 8a9e19f3c..daef2c6e3 100644 --- a/launchable/test_runners/jasmine.py +++ b/launchable/test_runners/jasmine.py @@ -19,14 +19,7 @@ def record_tests(client, reports): client.run() -@launchable.subset -def subset(client): - # read lines as test file names - for t in client.stdin(): - client.test_path(t.rstrip("\n")) - - client.run() - +subset = launchable.CommonSubsetImpls(__name__).scan_stdin() split_subset = launchable.CommonSplitSubsetImpls(__name__).split_subset() diff --git a/launchable/test_runners/launchable.py b/launchable/test_runners/launchable.py index df0907e36..250643c79 100644 --- a/launchable/test_runners/launchable.py +++ b/launchable/test_runners/launchable.py @@ -63,17 +63,35 @@ class CommonSubsetImpls: def __init__(self, module_name): self.cmdname = cmdname(module_name) - def scan_files(self, pattern): + def scan_stdin(self): + """ + Historical implementation of the files profile that's also used elsewhere. + Reads test one line at a time from stdin. Consider this implementation deprecated. + Newer test runners are advised to use scan_files() without the pattern argument. + """ + def subset(client): + # read lines as test file names + for t in client.stdin(): + client.test_path(t.rstrip("\n")) + client.run() + + return wrap(subset, subset_cmd, self.cmdname) + + def scan_files(self, pattern=None): """ Suitable for test runners that use files as unit of tests where file names follow a naming pattern. :param pattern: file masks that identify test files, such as '*_spec.rb' + for test runners that do not have natural file naming conventions, pass in None, + so that the implementation will refuse to accept directories. """ @click.argument('files', required=True, nargs=-1) def subset(client, files): # client type: Optimize in def lauchable.commands.subset.subset def parse(fname: str): if os.path.isdir(fname): + if pattern is None: + raise click.UsageError(f'{fname} is a directory, but expecting a file or GLOB') client.scan(fname, '**/' + pattern) elif fname == '@-': # read stdin diff --git a/launchable/test_runners/playwright.py b/launchable/test_runners/playwright.py index cd286e3a8..87e803213 100644 --- a/launchable/test_runners/playwright.py +++ b/launchable/test_runners/playwright.py @@ -51,14 +51,7 @@ def path_builder(case: TestCase, suite: TestSuite, client.run() -@launchable.subset -def subset(client): - # read lines as test file names - for t in client.stdin(): - client.test_path(t.rstrip("\n")) - - client.run() - +subset = launchable.CommonSubsetImpls(__name__).scan_stdin() split_subset = launchable.CommonSplitSubsetImpls(__name__).split_subset() diff --git a/launchable/test_runners/prove.py b/launchable/test_runners/prove.py index 67c83910a..f615034b9 100644 --- a/launchable/test_runners/prove.py +++ b/launchable/test_runners/prove.py @@ -14,13 +14,7 @@ def remove_leading_number_and_dash(input_string: str) -> str: return result -@launchable.subset -def subset(client): - # read lines as test file names - for t in client.stdin(): - client.test_path(t.rstrip("\n")) - - client.run() +subset = launchable.CommonSubsetImpls(__name__).scan_stdin() @click.argument('reports', required=True, nargs=-1) diff --git a/launchable/test_runners/vitest.py b/launchable/test_runners/vitest.py index d8731d5b5..523584300 100644 --- a/launchable/test_runners/vitest.py +++ b/launchable/test_runners/vitest.py @@ -28,10 +28,4 @@ def parse_func(report: str) -> ET.ElementTree: launchable.CommonRecordTestImpls.load_report_files(client=client, source_roots=reports) -@launchable.subset -def subset(client): - # read lines as test file names - for t in client.stdin(): - client.test_path(t.rstrip("\n")) - - client.run() +subset = launchable.CommonSubsetImpls(__name__).scan_stdin()