Skip to content

Commit 5d10144

Browse files
trishortsclaudepcruzparri
authored
test(ci): run external-service tests in a non-blocking job that skips on outage (#2682)
Introduce a shared convention for tests that hit a live external web service (UniProt now; Koina/PRIDE to follow). Such tests carry [Category("ExternalService")] and run in a dedicated CI job that is NOT a required check, so a third-party outage can never block a PR. The tests self-classify via ExternalServiceTestHelper.RunAsync: an availability failure (transport error, 5xx/429/408, or a service's HTTP-200 error body such as UniProt's "Error encountered when streaming data") is reported as Skipped with a plain-English reason on the CI log ("we tried, the service is down, don't worry"), while a genuine contract break (nothing parses, URL rejected) still fails. - Add ExternalServiceTestHelper (RunAsync + ThrowIfUnavailable) and the ExternalServiceUnavailableException marker. - Convert GuiFunctionsTest.UniProtDownloadCanary from [Explicit] to [Category("ExternalService")] + the helper; both cases use uncompressed responses so outage detection is reliable. - Test.yml: exclude Category=ExternalService from the required unit-test run and add the non-blocking external-service-tests job. Note for maintainers: leave external-service-tests OUT of branch-protection required checks so it stays informational. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: pcruzparri <43578034+pcruzparri@users.noreply.github.com>
1 parent 54cf6f5 commit 5d10144

3 files changed

Lines changed: 212 additions & 49 deletions

File tree

‎.github/workflows/Test.yml‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,36 @@ jobs:
2727
run: dotnet add ./MetaMorpheus/Test/Test.csproj package coverlet.collector -v 6.0.2
2828

2929
- name: Run unit tests with coverage
30-
run: dotnet test .\MetaMorpheus\Test\Test.csproj --configuration Release --verbosity normal --collect:"XPlat Code Coverage"
31-
30+
# Exclude live external-service tests (UniProt, etc.) from the required run; they run in the
31+
# separate, non-blocking external-service-tests job below.
32+
run: dotnet test .\MetaMorpheus\Test\Test.csproj --configuration Release --verbosity normal --filter "Category!=ExternalService" --collect:"XPlat Code Coverage"
33+
3234
- name: Upload coverage to Codecov
3335
uses: codecov/codecov-action@v4
3436
with:
3537
token: ${{ secrets.CODECOV_TOKEN }}
3638
verbose: true
3739
flags: unittests
40+
41+
# Runs tests that depend on live external web services (UniProt, ...). Kept separate and
42+
# intentionally NOT a required status check, so a third-party outage never blocks a PR. The tests
43+
# self-classify: an outage is reported as Skipped (Assert.Ignore) and only a genuine contract
44+
# break fails this job. Do NOT add this job to branch-protection required checks.
45+
external-service-tests:
46+
name: External-service tests (non-blocking)
47+
runs-on: windows-latest
48+
49+
steps:
50+
- uses: actions/checkout@v4
51+
- name: Setup .NET
52+
uses: actions/setup-dotnet@v4
53+
with:
54+
dotnet-version: 8.0.204
55+
- name: Restore dependencies
56+
run: dotnet restore ./MetaMorpheus/MetaMorpheus.sln
57+
58+
- name: Build
59+
run: dotnet build --no-restore ./MetaMorpheus/MetaMorpheus.sln --configuration Release
60+
61+
- name: Run external-service tests
62+
run: dotnet test .\MetaMorpheus\Test\Test.csproj --configuration Release --no-build --verbosity normal --filter "Category=ExternalService"

‎MetaMorpheus/Test/ExternalServiceTestHelper.cs‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
using System;
2+
using System.Net.Http;
3+
using System.Net.Sockets;
4+
using System.Threading.Tasks;
5+
using NUnit.Framework;
6+
7+
namespace Test
8+
{
9+
/// <summary>
10+
/// Support for tests that exercise a live external web service (UniProt, Koina, PRIDE, ...).
11+
/// Such tests carry <c>[Category("ExternalService")]</c> so CI can run them in a dedicated,
12+
/// non-blocking job (see .github/workflows/Test.yml) instead of the required unit-test run.
13+
///
14+
/// The hard part is telling two failures apart:
15+
/// * the service is unavailable (down, rate-limited, 5xx, timeout) - NOT our bug, so the
16+
/// test should be <b>skipped</b> ("we tried, the service is down, don't worry"); versus
17+
/// * the service answered but the contract is broken (our URL is wrong, the response no
18+
/// longer parses, an expected value is missing) - that is a real regression and must FAIL.
19+
///
20+
/// <see cref="RunAsync"/> wraps a test body and converts availability failures into
21+
/// <see cref="Assert.Ignore(string)"/> (reported as Skipped) while letting genuine assertion
22+
/// failures propagate. A test signals "unavailable" from inside the body either by letting a
23+
/// transport exception bubble up or by calling <see cref="ThrowIfUnavailable"/> on the response.
24+
/// </summary>
25+
public static class ExternalServiceTestHelper
26+
{
27+
/// <summary>
28+
/// Runs <paramref name="testBody"/>; if the external service proves unavailable, marks the
29+
/// test Skipped (via Assert.Ignore) instead of Failed. Real assertion failures propagate.
30+
/// </summary>
31+
public static async Task RunAsync(string serviceName, Func<Task> testBody)
32+
{
33+
try
34+
{
35+
await testBody();
36+
}
37+
catch (ExternalServiceUnavailableException e)
38+
{
39+
Skip(serviceName, $"unavailable ({e.Message})");
40+
}
41+
catch (HttpRequestException e)
42+
{
43+
Skip(serviceName, $"unavailable ({e.Message})");
44+
}
45+
catch (TaskCanceledException)
46+
{
47+
Skip(serviceName, "timed out");
48+
}
49+
catch (SocketException e)
50+
{
51+
Skip(serviceName, $"unreachable ({e.Message})");
52+
}
53+
}
54+
55+
/// <summary>
56+
/// Records the skip reason on both the console/CI log (via TestContext.Progress, which is
57+
/// flushed immediately regardless of verbosity) and the NUnit test result, then skips the
58+
/// test. This is what surfaces the "we tried, the service is down, don't worry" message.
59+
/// </summary>
60+
private static void Skip(string serviceName, string reason)
61+
{
62+
string message = $"Skipping external-service test: {serviceName} {reason}. " +
63+
"This is a third-party availability problem, not a code failure.";
64+
TestContext.Progress.WriteLine(message);
65+
Assert.Ignore(message);
66+
}
67+
68+
/// <summary>
69+
/// Probes an external service's health/readiness URL and skips the calling test (or, from a
70+
/// [OneTimeSetUp], the whole fixture) if it is unreachable. Use this for tests whose service
71+
/// call is buried deep in production code and cannot easily route through <see cref="RunAsync"/>.
72+
/// </summary>
73+
public static void EnsureReachable(string serviceName, string healthUrl)
74+
{
75+
try
76+
{
77+
using var client = new HttpClient { Timeout = TimeSpan.FromSeconds(20) };
78+
var response = client.GetAsync(healthUrl).GetAwaiter().GetResult();
79+
ThrowIfUnavailable(response);
80+
if (!response.IsSuccessStatusCode)
81+
{
82+
throw new ExternalServiceUnavailableException($"health check returned HTTP {(int)response.StatusCode}");
83+
}
84+
}
85+
catch (ExternalServiceUnavailableException e)
86+
{
87+
Skip(serviceName, $"unavailable ({e.Message})");
88+
}
89+
catch (HttpRequestException e)
90+
{
91+
Skip(serviceName, $"unavailable ({e.Message})");
92+
}
93+
catch (TaskCanceledException)
94+
{
95+
Skip(serviceName, "timed out");
96+
}
97+
catch (SocketException e)
98+
{
99+
Skip(serviceName, $"unreachable ({e.Message})");
100+
}
101+
}
102+
103+
/// <summary>
104+
/// Classifies an HTTP response as a service-availability problem and, if so, throws
105+
/// <see cref="ExternalServiceUnavailableException"/> so <see cref="RunAsync"/> skips the test.
106+
/// Catches transport-level status codes (408/429/5xx) and known error bodies that some
107+
/// services (e.g. UniProt's streaming endpoint) return with an HTTP 200.
108+
/// </summary>
109+
public static void ThrowIfUnavailable(HttpResponseMessage response, string bodyText = null)
110+
{
111+
int status = (int)response.StatusCode;
112+
if (status == 408 || status == 429 || status >= 500)
113+
{
114+
throw new ExternalServiceUnavailableException($"HTTP {status} {response.ReasonPhrase}");
115+
}
116+
117+
// Some services answer 200 but stream an error message in the body.
118+
if (!string.IsNullOrWhiteSpace(bodyText) &&
119+
bodyText.Contains("Error encountered when streaming data", StringComparison.OrdinalIgnoreCase))
120+
{
121+
throw new ExternalServiceUnavailableException("service returned an error body instead of data");
122+
}
123+
}
124+
}
125+
126+
/// <summary>
127+
/// Marker exception meaning "the external service is unavailable" (as opposed to a real bug).
128+
/// <see cref="ExternalServiceTestHelper.RunAsync"/> turns this into a skipped test.
129+
/// </summary>
130+
public class ExternalServiceUnavailableException : Exception
131+
{
132+
public ExternalServiceUnavailableException(string message) : base(message) { }
133+
}
134+
}

‎MetaMorpheus/Test/GuiTests/GuiFunctionsTest.cs‎

Lines changed: 51 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -94,56 +94,60 @@ public static void LoadUniProtProteomeFromLocalFile_GeneratesTargetsAndDecoys(st
9494
}
9595

9696
// Live canary for the UniProt REST download used by the "Download UniProt Database" GUI.
97-
// Marked [Explicit] (and Category "UniProt") so it never runs in an unfiltered `dotnet test`
98-
// pass - i.e. it stays out of CI, exactly like the [Category("Koina")] network tests - and
99-
// only fires when a developer selects it deliberately. Run it to confirm that
100-
// rest.uniprot.org is reachable, that GetUniProtHtmlQueryString still yields a working URL,
101-
// and that ProteinDbLoader can parse the response. The assertion is intentionally loose
102-
// (proteins parsed > 0) so a UniProt content revision does not turn this into a red herring;
103-
// its job is to detect that the pipeline is *working*, not to pin an exact entry count.
97+
// Tagged [Category("ExternalService")] (+ "UniProt") so it runs in the dedicated, non-blocking
98+
// external-service CI job rather than the required unit-test run. It confirms rest.uniprot.org
99+
// is reachable, that GetUniProtHtmlQueryString still yields a working URL, and that
100+
// ProteinDbLoader can parse the response. Via ExternalServiceTestHelper.RunAsync, a UniProt
101+
// outage (transport error, 5xx, or the HTTP-200 "Error encountered when streaming data" body)
102+
// is reported as Skipped rather than Failed - "we tried, UniProt was down, don't worry" - while
103+
// a genuine contract break (nothing parses, URL rejected) still fails. The assertion is
104+
// intentionally loose (proteins parsed > 0) so a UniProt content revision is not a red herring.
104105
// UP000001207 = Bacillus phage phi29 (small reference proteome).
105-
[Test, Explicit]
106+
[Test]
107+
[Category("ExternalService")]
106108
[Category("UniProt")]
107-
[TestCase("UP000001207", true, false, true, true)] // reviewed, XML, compressed
108-
[TestCase("UP000001207", false, false, false, false)] // all, FASTA, uncompressed
109-
public static async Task UniProtDownloadCanary(string proteomeID, bool reviewed, bool isoforms, bool xmlFormat, bool compressed)
110-
{
111-
var proteomeURL = DownloadUniProtDatabaseFunctions.GetUniProtHtmlQueryString(proteomeID, reviewed,
112-
isoforms, xmlFormat, compressed);
113-
114-
var extension = (xmlFormat ? ".xml" : ".fasta") + (compressed ? ".gz" : "");
115-
var filePath = Path.Combine(TestContext.CurrentContext.TestDirectory, $@"DatabaseTests\uniprot_canary{extension}");
116-
117-
HttpClientHandler handler = new HttpClientHandler(); // without this, the download is very slow
118-
handler.Proxy = null;
119-
handler.UseProxy = false;
120-
121-
using var client = new HttpClient(handler); // client for using the REST Api
122-
var response = await client.GetAsync(proteomeURL);
123-
124-
using (var file = File.Create(filePath)) // saves the file
125-
{
126-
var content = await response.Content.ReadAsStreamAsync();
127-
await content.CopyToAsync(file);
128-
}
129-
130-
List<Protein> reader;
131-
if (xmlFormat)
109+
[TestCase("UP000001207", true, false, true, false)] // reviewed, XML
110+
[TestCase("UP000001207", false, false, false, false)] // all, FASTA
111+
public static Task UniProtDownloadCanary(string proteomeID, bool reviewed, bool isoforms, bool xmlFormat, bool compressed) =>
112+
ExternalServiceTestHelper.RunAsync("UniProt", async () =>
132113
{
133-
reader = ProteinDbLoader.LoadProteinXML(proteinDbLocation: filePath, generateTargets: true, decoyType: DecoyType.Reverse,
134-
allKnownModifications: null, isContaminant: false, modTypesToExclude: null, out _, maxHeterozygousVariants: 0);
135-
}
136-
else
137-
{
138-
reader = ProteinDbLoader.LoadProteinFasta(filePath, generateTargets: true, decoyType: DecoyType.Reverse,
139-
isContaminant: false, out _);
140-
}
141-
142-
File.Delete(filePath);
143-
144-
Assert.That(reader.Count, Is.GreaterThan(0),
145-
"UniProt returned no parseable proteins - the REST endpoint may be down or the query URL/format has changed.");
146-
}
114+
var proteomeURL = DownloadUniProtDatabaseFunctions.GetUniProtHtmlQueryString(proteomeID, reviewed,
115+
isoforms, xmlFormat, compressed);
116+
117+
var extension = (xmlFormat ? ".xml" : ".fasta") + (compressed ? ".gz" : "");
118+
var filePath = Path.Combine(TestContext.CurrentContext.TestDirectory, $@"DatabaseTests\uniprot_canary{extension}");
119+
120+
HttpClientHandler handler = new HttpClientHandler(); // without this, the download is very slow
121+
handler.Proxy = null;
122+
handler.UseProxy = false;
123+
124+
using var client = new HttpClient(handler); // client for using the REST Api
125+
var response = await client.GetAsync(proteomeURL);
126+
127+
var bytes = await response.Content.ReadAsByteArrayAsync();
128+
// Sniff a text prefix (harmless on gzipped payloads) so a service outage is skipped, not failed.
129+
var textPreview = System.Text.Encoding.UTF8.GetString(bytes, 0, Math.Min(bytes.Length, 512));
130+
ExternalServiceTestHelper.ThrowIfUnavailable(response, textPreview);
131+
132+
File.WriteAllBytes(filePath, bytes); // saves the file
133+
134+
List<Protein> reader;
135+
if (xmlFormat)
136+
{
137+
reader = ProteinDbLoader.LoadProteinXML(proteinDbLocation: filePath, generateTargets: true, decoyType: DecoyType.Reverse,
138+
allKnownModifications: null, isContaminant: false, modTypesToExclude: null, out _, maxHeterozygousVariants: 0);
139+
}
140+
else
141+
{
142+
reader = ProteinDbLoader.LoadProteinFasta(filePath, generateTargets: true, decoyType: DecoyType.Reverse,
143+
isContaminant: false, out _);
144+
}
145+
146+
File.Delete(filePath);
147+
148+
Assert.That(reader.Count, Is.GreaterThan(0),
149+
"UniProt returned no parseable proteins - the REST endpoint may be down or the query URL/format has changed.");
150+
});
147151

148152
[Test]
149153
public static void TestFileLoadingWithDuplicateFiles()

0 commit comments

Comments
 (0)