-
Notifications
You must be signed in to change notification settings - Fork 399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow insecure requests on debug builds via OSU_INSECURE_REQUESTS
environment variable
#6283
Changes from all commits
ba35057
426addc
597e6d6
c6e29bd
c035127
d418fe8
70f3c54
694f7f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,75 @@ | ||
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence. | ||
// See the LICENCE file in the repository root for full licence text. | ||
|
||
using System; | ||
using NUnit.Framework; | ||
using NUnit.Framework.Interfaces; | ||
using NUnit.Framework.Internal; | ||
using NUnit.Framework.Internal.Commands; | ||
|
||
namespace osu.Framework.Tests | ||
{ | ||
/// <summary> | ||
/// An attribute to mark any flaky tests. | ||
/// Will add a retry count unless environment variable `FAIL_FLAKY_TESTS` is set to `1`. | ||
/// </summary> | ||
public class FlakyTestAttribute : RetryAttribute | ||
[AttributeUsage(AttributeTargets.Method, Inherited = false)] | ||
public class FlakyTestAttribute : NUnitAttribute, IRepeatTest | ||
{ | ||
private readonly int tryCount; | ||
|
||
public FlakyTestAttribute() | ||
: this(10) | ||
{ | ||
} | ||
|
||
public FlakyTestAttribute(int tryCount) | ||
: base(FrameworkEnvironment.FailFlakyTests ? 1 : tryCount) | ||
{ | ||
this.tryCount = tryCount; | ||
} | ||
|
||
public TestCommand Wrap(TestCommand command) => new FlakyTestCommand(command, tryCount); | ||
|
||
// Adapted from https://github.com/nunit/nunit/blob/4eaab2eef3713907ca37bfb2f7f47e3fc2785214/src/NUnitFramework/framework/Attributes/RetryAttribute.cs | ||
// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt | ||
public class FlakyTestCommand : DelegatingTestCommand | ||
{ | ||
private readonly int tryCount; | ||
|
||
public FlakyTestCommand(TestCommand innerCommand, int tryCount) | ||
: base(innerCommand) | ||
{ | ||
this.tryCount = tryCount; | ||
} | ||
|
||
public override TestResult Execute(TestExecutionContext context) | ||
{ | ||
int count = FrameworkEnvironment.FailFlakyTests ? 1 : tryCount; | ||
|
||
while (count-- > 0) | ||
{ | ||
try | ||
{ | ||
context.CurrentResult = innerCommand.Execute(context); | ||
} | ||
catch (Exception ex) | ||
{ | ||
context.CurrentResult ??= context.CurrentTest.MakeTestResult(); | ||
context.CurrentResult.RecordException(ex); | ||
} | ||
|
||
if (context.CurrentResult.ResultState != ResultState.Failure) | ||
break; | ||
|
||
if (count > 0) | ||
{ | ||
context.CurrentResult = context.CurrentTest.MakeTestResult(); | ||
context.CurrentRepeatCount++; | ||
} | ||
} | ||
|
||
return context.CurrentResult; | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence. | ||
// See the LICENCE file in the repository root for full licence text. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using NUnit.Framework; | ||
using osu.Framework.Extensions; | ||
using osu.Framework.IO.Stores; | ||
|
||
namespace osu.Framework.Tests.IO | ||
{ | ||
[TestFixture] | ||
[Category("httpbin")] | ||
public class TestOnlineStore | ||
{ | ||
private const string default_protocol = "http"; | ||
|
||
private static readonly string host; | ||
private static readonly IEnumerable<string> protocols; | ||
|
||
private bool oldAllowInsecureRequests; | ||
private OnlineStore store = null!; | ||
|
||
static TestOnlineStore() | ||
{ | ||
bool localHttpBin = Environment.GetEnvironmentVariable("OSU_TESTS_LOCAL_HTTPBIN") == "1"; | ||
|
||
if (localHttpBin) | ||
{ | ||
// httpbin very frequently falls over and causes random tests to fail | ||
// Thus github actions builds rely on a local httpbin instance to run the tests | ||
|
||
host = "127.0.0.1:8080"; | ||
protocols = new[] { default_protocol }; | ||
} | ||
else | ||
{ | ||
host = "httpbin.org"; | ||
protocols = new[] { default_protocol, "https" }; | ||
} | ||
} | ||
|
||
[OneTimeSetUp] | ||
public void GlobalSetup() | ||
{ | ||
oldAllowInsecureRequests = FrameworkEnvironment.AllowInsecureRequests; | ||
FrameworkEnvironment.AllowInsecureRequests = true; | ||
} | ||
|
||
[OneTimeTearDown] | ||
public void GlobalTeardown() | ||
{ | ||
FrameworkEnvironment.AllowInsecureRequests = oldAllowInsecureRequests; | ||
} | ||
|
||
[SetUp] | ||
public void Setup() | ||
{ | ||
store = new OnlineStore(); | ||
} | ||
|
||
[Test, Retry(5)] | ||
public void TestValidUrlReturnsData([ValueSource(nameof(protocols))] string protocol, [Values(true, false)] bool async) | ||
{ | ||
byte[]? result = async | ||
? store.GetAsync($"{protocol}://{host}/image/png").GetResultSafely() | ||
: store.Get($"{protocol}://{host}/image/png"); | ||
|
||
Assert.That(result, Is.Not.Null); | ||
Assert.That(result, Has.Length.GreaterThan(0)); | ||
} | ||
|
||
[Test] | ||
public void TestMissingSchemeReturnsNull([Values(true, false)] bool async) | ||
{ | ||
byte[]? result = async | ||
? store.GetAsync($"{host}/image/png").GetResultSafely() | ||
: store.Get($"{host}/image/png"); | ||
|
||
Assert.That(result, Is.Null); | ||
} | ||
|
||
[Test] | ||
public void TestInvalidUrlReturnsNull() | ||
{ | ||
byte[]? result = store.Get("this is not a valid url"); | ||
Assert.That(result, Is.Null); | ||
} | ||
|
||
[Test] | ||
public void TestNullUrlReturnsNull() | ||
{ | ||
// Not sure if this store should accept a null URL, but let's test it anyway. | ||
byte[]? result = store.Get(null); | ||
Assert.That(result, Is.Null); | ||
} | ||
|
||
[Test] | ||
public void TestFileUrlFails([Values(true, false)] bool async) | ||
{ | ||
// Known, guaranteed file path. | ||
string path = new Uri(RuntimeInfo.EntryAssembly.Location).AbsoluteUri; | ||
Check warning on line 102 in osu.Framework.Tests/IO/TestOnlineStore.cs
|
||
|
||
byte[]? result = async | ||
? store.GetAsync(path).GetResultSafely() | ||
: store.Get(path); | ||
|
||
Assert.That(result, Is.Null); | ||
} | ||
|
||
[Test] | ||
public void TestBadWebRequest([ValueSource(nameof(protocols))] string protocol, [Values(true, false)] bool async) | ||
{ | ||
byte[]? result = async | ||
? store.GetAsync($"{protocol}://{host}/status/500").GetResultSafely() | ||
: store.Get($"{protocol}://{host}/status/500"); | ||
|
||
Assert.That(result, Is.Null); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd hope for test coverage of |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels a bit weird no?
... like I'd have to check how the tests are being run as to whether this is getting full test coverage or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is copied from
TestWebRequest
, so I can't provide much other than this is how it's been done so far. Do you have any suggestions?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried changing this to skip the "https" tests rather than not list them at all, but it's only marginally better. I don't have any better ideas so let's let this one pass.