diff --git a/smart_tests/commands/gate.py b/smart_tests/commands/gate.py index 2eafe96c5..b91ef8b5b 100644 --- a/smart_tests/commands/gate.py +++ b/smart_tests/commands/gate.py @@ -1,5 +1,7 @@ import json +import os import sys +import uuid from http import HTTPStatus from typing import Annotated @@ -12,6 +14,7 @@ from .. import args4p from ..app import Application from ..args4p import typer +from ..testpath import unparse_test_path from ..utils.commands import Command from ..utils.session import SessionId from ..utils.smart_tests_client import SmartTestsClient @@ -21,8 +24,8 @@ def gate(app_instance: Application, session: Annotated[SessionId, SessionId.as_option()], is_json_format: Annotated[bool, typer.Option( - "--json", - help="display JSON format")] = False): + "--json", + help="display JSON format")] = False): tracking_client = TrackingClient(Command.GATE, app=app_instance) client = SmartTestsClient(tracking_client=tracking_client, app=app_instance) try: @@ -50,6 +53,10 @@ def gate(app_instance: Application, client.print_exception_and_recover(e, "Warning: failed to fetch gate status") +def _escape_github_actions_command_value(value: str) -> str: + return value.replace('\r', '%0D').replace('\n', '%0A') + + def display_as_json(res: Response): res_json = res.json() click.echo(json.dumps(res_json, indent=2)) @@ -68,3 +75,27 @@ def display_as_table(res: Response): ]] click.echo(tabulate(rows, headers, tablefmt="github")) + + failed_tests = res_json.get('actionableFailedTests', []) + is_github_actions = os.getenv('GITHUB_ACTIONS') + if failed_tests: + click.echo("\nActionable Failure Details:\n") + for i, test in enumerate(failed_tests, 1): + test_path = unparse_test_path(test.get("testPath", [])) + stderr = (test.get("stderr") or "").strip() + if is_github_actions: + safe_test_path = _escape_github_actions_command_value(test_path) + token = uuid.uuid4().hex + click.echo("::group::{}. {}".format(i, safe_test_path)) + click.echo("::stop-commands::{}".format(token)) + if stderr: + for line in stderr.splitlines(): + click.echo(line) + click.echo("::{}::".format(token)) + click.echo("::endgroup::") + else: + click.echo("{}. {}".format(i, test_path)) + if stderr: + for line in stderr.splitlines(): + click.echo(" {}".format(line)) + click.echo("") diff --git a/tests/commands/test_gate.py b/tests/commands/test_gate.py index 5fb8458dd..8b5c5870e 100644 --- a/tests/commands/test_gate.py +++ b/tests/commands/test_gate.py @@ -22,7 +22,8 @@ def test_gate_passed(self): json={ 'status': 'PASSED', 'quarantinedFailures': 5, - 'actionableFailures': 0 + 'actionableFailures': 0, + 'actionableFailedTests': [] }, status=200) @@ -43,13 +44,24 @@ def test_gate_failed(self): json={ 'status': 'FAILED', 'quarantinedFailures': 2, - 'actionableFailures': 3 + 'actionableFailures': 1, + 'actionableFailedTests': [ + { + 'testPath': [ + {'type': 'file', 'name': 'src/FooTest.java'}, + {'type': 'testcase', 'name': 'testBar'} + ], + 'stderr': 'AssertionError: expected true but was false' + } + ] }, status=200) result = self.cli('gate', '--session', self.session) self.assert_exit_code(result, 1) self.assertIn('FAILED', result.output) + self.assertIn('file=src/FooTest.java#testcase=testBar', result.output) + self.assertIn('AssertionError: expected true but was false', result.output) @responses.activate @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) @@ -58,7 +70,8 @@ def test_gate_passed_json_format(self): gate_data = { 'status': 'PASSED', 'quarantinedFailures': 5, - 'actionableFailures': 0 + 'actionableFailures': 0, + 'actionableFailedTests': [] } responses.add( @@ -86,7 +99,16 @@ def test_gate_failed_json_format(self): gate_data = { 'status': 'FAILED', 'quarantinedFailures': 2, - 'actionableFailures': 3 + 'actionableFailures': 1, + 'actionableFailedTests': [ + { + 'testPath': [ + {'type': 'file', 'name': 'src/FooTest.java'}, + {'type': 'testcase', 'name': 'testBar'} + ], + 'stderr': 'AssertionError: expected true but was false' + } + ] } responses.add( @@ -105,7 +127,97 @@ def test_gate_failed_json_format(self): output_json = json.loads(result.output) self.assertEqual(output_json['status'], 'FAILED') self.assertEqual(output_json['quarantinedFailures'], 2) - self.assertEqual(output_json['actionableFailures'], 3) + self.assertEqual(output_json['actionableFailures'], 1) + self.assertEqual(len(output_json['actionableFailedTests']), 1) + self.assertEqual(output_json['actionableFailedTests'][0]['testPath'][0]['name'], 'src/FooTest.java') + + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token, "GITHUB_ACTIONS": "true"}) + def test_gate_failed_github_actions_format(self): + """Test gate command uses ::group:: syntax when running in GitHub Actions""" + responses.add( + responses.GET, + "{}/intake/organizations/{}/workspaces/{}/gate".format( + get_base_url(), + self.organization, + self.workspace), + json={ + 'status': 'FAILED', + 'quarantinedFailures': 0, + 'actionableFailures': 1, + 'actionableFailedTests': [ + { + 'testPath': [ + {'type': 'file', 'name': 'src/FooTest.java'}, + {'type': 'testcase', 'name': 'testBar'} + ], + 'stderr': 'AssertionError: expected true but was false' + } + ] + }, + status=200) + + result = self.cli('gate', '--session', self.session) + self.assert_exit_code(result, 1) + self.assertIn('::group::1. file=src/FooTest.java#testcase=testBar', result.output) + self.assertIn('::stop-commands::', result.output) + self.assertIn('AssertionError: expected true but was false', result.output) + self.assertIn('::endgroup::', result.output) + + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token, "GITHUB_ACTIONS": "true"}) + def test_gate_github_actions_stderr_with_command_syntax(self): + """Test that stderr containing ::patterns:: is safely wrapped with stop-commands""" + responses.add( + responses.GET, + "{}/intake/organizations/{}/workspaces/{}/gate".format( + get_base_url(), + self.organization, + self.workspace), + json={ + 'status': 'FAILED', + 'quarantinedFailures': 0, + 'actionableFailures': 1, + 'actionableFailedTests': [ + { + 'testPath': [ + {'type': 'file', 'name': 'src/FooTest.java'}, + {'type': 'testcase', 'name': 'testBar'} + ], + 'stderr': ( + '::error::some error\n' + '::warning::spoofed\n' + '::add-mask::secret-value\n' + '::set-output name=x::y\n' + 'java.lang.AssertionError' + ) + } + ] + }, + status=200) + + result = self.cli('gate', '--session', self.session) + self.assert_exit_code(result, 1) + + # verify all dangerous commands are sandwiched between stop-commands and resume token + stop_idx = result.output.index('::stop-commands::') + # resume token is the line between stop-commands and ::endgroup:: + endgroup_idx = result.output.index('::endgroup::') + + error_idx = result.output.index('::error::some error') + warning_idx = result.output.index('::warning::spoofed') + mask_idx = result.output.index('::add-mask::secret-value') + assertion_idx = result.output.index('java.lang.AssertionError') + + # all stderr content must be after ::stop-commands:: and before ::endgroup:: + self.assertLess(stop_idx, error_idx) + self.assertLess(stop_idx, warning_idx) + self.assertLess(stop_idx, mask_idx) + self.assertLess(stop_idx, assertion_idx) + self.assertLess(error_idx, endgroup_idx) + self.assertLess(warning_idx, endgroup_idx) + self.assertLess(mask_idx, endgroup_idx) + self.assertLess(assertion_idx, endgroup_idx) @responses.activate @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token})