From 2c021aec1909e0a714794040c56f5004b867755f Mon Sep 17 00:00:00 2001 From: Carlos Date: Sun, 11 Jan 2026 11:08:51 +0100 Subject: [PATCH] fix: Include target_url in GitHub status for timed-out tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a test times out due to exceeding the maximum runtime limit, the GitHub status was set to ERROR but without a target_url. This made it impossible for users to click through to see what happened - they would just see "ERROR" with no link. Root cause: In delete_expired_instances(), update_status_on_github() was called without the target_url parameter, which resulted in GitHub displaying a null/empty URL. Fix: Build the target_url (using url_for with fallback for non-request context) and pass it to update_status_on_github() so users can navigate to the test page to see results even when a test times out. Discovered while investigating why CCExtractor/ccextractor#2007 Linux test showed ERROR on GitHub with no clickable link. Test #7736 had run for ~4 hours, executed 116 regression tests (105 failed), but was canceled due to time limit. The test page existed and showed results, but GitHub had no link to it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- mod_ci/controllers.py | 9 ++++++- tests/test_ci/test_controllers.py | 39 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/mod_ci/controllers.py b/mod_ci/controllers.py index 5cd449fe..9e8ea5ec 100755 --- a/mod_ci/controllers.py +++ b/mod_ci/controllers.py @@ -471,7 +471,14 @@ def delete_expired_instances(compute, max_runtime, project, zone, db, repository gh_commit = repository.get_commit(test.commit) if gh_commit is not None: - update_status_on_github(gh_commit, Status.ERROR, message, f"CI - {platform_name}") + # Build target_url so users can see test results + from flask import url_for + try: + target_url = url_for('test.by_id', test_id=test_id, _external=True) + except RuntimeError: + # Outside of request context + target_url = f"https://sampleplatform.ccextractor.org/test/{test_id}" + update_status_on_github(gh_commit, Status.ERROR, message, f"CI - {platform_name}", target_url) # Delete VM instance with tracking for verification from run import log diff --git a/tests/test_ci/test_controllers.py b/tests/test_ci/test_controllers.py index f28e8398..d74303a0 100644 --- a/tests/test_ci/test_controllers.py +++ b/tests/test_ci/test_controllers.py @@ -1517,6 +1517,45 @@ def test_delete_expired_instances_db_commit_failure( # Should continue to next instance after commit failure mock_delete.assert_not_called() + @mock.patch('mod_ci.controllers.delete_instance_with_tracking') + @mock.patch('mod_ci.controllers.update_status_on_github') + @mock.patch('mod_ci.controllers.safe_db_commit') + @mock.patch('mod_ci.controllers.is_instance_testing') + @mock.patch('run.log') + def test_delete_expired_instances_includes_target_url( + self, mock_log, mock_is_testing, mock_safe_commit, + mock_update_github, mock_delete_tracking): + """Test delete_expired_instances includes target_url in GitHub status.""" + from mod_ci.controllers import delete_expired_instances + + mock_is_testing.return_value = True + mock_safe_commit.return_value = True + mock_delete_tracking.return_value = {'name': 'op-123'} + + # Create a mock compute service with expired instance + mock_compute = MagicMock() + mock_compute.instances().list().execute.return_value = { + 'items': [{ + 'name': 'linux-1', + 'creationTimestamp': '2020-01-01T00:00:00.000+00:00' + }] + } + + mock_repo = MagicMock() + mock_gh_commit = MagicMock() + mock_repo.get_commit.return_value = mock_gh_commit + + delete_expired_instances( + mock_compute, 60, 'project', 'zone', g.db, mock_repo) + + # Verify update_status_on_github was called with target_url + mock_update_github.assert_called_once() + call_args = mock_update_github.call_args + # Check that target_url (5th argument) contains the test ID + self.assertEqual(len(call_args[0]), 5) # 5 positional args + target_url = call_args[0][4] + self.assertIn('/test/1', target_url) + @mock.patch('github.Github.get_repo') @mock.patch('requests.get', side_effect=mock_api_request_github) @mock.patch('mod_ci.controllers.inform_mailing_list')