From ec416615be4b2a66851a589bb2220d084adbc1c8 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 17 Sep 2026 00:27:58 +0100 Subject: [PATCH 1/4] TEST: Ask only for the settings a test actually uses The assembly level setup read CLOUD_ROOT_URL before anything ran, so a run with that variable unset failed every test, whatever it needed. The Contract tests then asked for PAID_RESOURCE_KEY as well, although an example that is already running was pointed at its cloud and given its key by whoever started it. An on-premise example has neither value, so the callers were passing placeholders to get a run started. The cloud URL is now read where it is used, and the options for starting an example are built in one place, ExampleApps.BuildOptions. An example this suite launches still needs both values and fails naming the one that is missing. An example that is already running needs neither and is passed whatever is set. Also in this change, because they are in the same files: - the python example is launched through the interpreter inside its virtual environment, which is Scripts/python.exe on Windows and bin/python elsewhere, so the launch no longer assumes bin/python; - the comment about which examples render a device id server-side said java and rust render none. The rust example renders one, in the same table as the device type, so only java is named now. The check itself is unchanged, as it reads the id where it is rendered rather than demanding it everywhere. ContractSettingsTests covers the rules by case. They read through a lookup the test supplies, so they do not change the environment of the run around them. --- BrowserCache/SessionStorageCacheTests.cs | 26 +-- ...ampleClientSideOverridePageContentTests.cs | 21 +- .../ExampleClientSideOverrideTests.cs | 21 +- ClientSideOverrides/ExampleRenderTests.cs | 29 +-- Examples/ContractSettingsTests.cs | 186 ++++++++++++++++++ Examples/ExampleApps.cs | 89 ++++++++- TestInitialiser.cs | 28 ++- TestsCommon/Helpers/TestHelpers.cs | 16 +- TestsCommon/TestConfig.cs | 63 ++++-- 9 files changed, 356 insertions(+), 123 deletions(-) create mode 100644 Examples/ContractSettingsTests.cs diff --git a/BrowserCache/SessionStorageCacheTests.cs b/BrowserCache/SessionStorageCacheTests.cs index edce475..8849b33 100644 --- a/BrowserCache/SessionStorageCacheTests.cs +++ b/BrowserCache/SessionStorageCacheTests.cs @@ -1,6 +1,5 @@ using FiftyOne.Pipeline.Cloud.SeleniumTests.Examples; using FiftyOne.Pipeline.Cloud.SeleniumTests.Helpers; -using FiftyOne.Pipeline.Cloud.Tests.Common; using FiftyOne.Pipeline.Cloud.Tests.Common.Helpers; using Microsoft.VisualStudio.TestTools.UnitTesting; using OpenQA.Selenium; @@ -41,31 +40,20 @@ public class SessionStorageCacheTests [ClassInitialize] public static void ClassInit(TestContext context) { - string rootUrl; - string resourceKey; - try - { - rootUrl = TestConfig.Instance().RootUrl; - resourceKey = TestConfig.Instance().PaidResourceKey; - } - catch (InvalidOperationException ex) - { - Assert.Inconclusive(ex.Message); - return; - } - if (!ExampleApps.TryCreate(out _app, out var skipReason)) { Assert.Inconclusive(skipReason); return; } + // An example that is already running (EXAMPLE_URL) was pointed at + // its cloud and given its key by whoever started it, so neither is + // required here. One this suite launches needs both, and a missing + // one fails this class naming the variable. + var options = ExampleApps.BuildOptions( + _app, TestHelpers.GetRandomUnusedPort()); + _appTokenSource = new CancellationTokenSource(); - var options = new ExampleAppOptions( - TestHelpers.GetRandomUnusedPort(), - new Uri(rootUrl), - resourceKey, - new Dictionary()); _app.StartAsync(options, _appTokenSource.Token) .GetAwaiter().GetResult(); } diff --git a/ClientSideOverrides/ExampleClientSideOverridePageContentTests.cs b/ClientSideOverrides/ExampleClientSideOverridePageContentTests.cs index 71eaec8..54bb419 100644 --- a/ClientSideOverrides/ExampleClientSideOverridePageContentTests.cs +++ b/ClientSideOverrides/ExampleClientSideOverridePageContentTests.cs @@ -6,7 +6,6 @@ using FiftyOne.Pipeline.Cloud.SeleniumTests.Examples; using FiftyOne.Pipeline.Cloud.SeleniumTests.Helpers; using FiftyOne.Pipeline.Cloud.Tests.Common.Helpers; -using FiftyOne.Pipeline.Cloud.Tests.Common.TestElements; using Microsoft.VisualStudio.TestTools.UnitTesting; using OpenQA.Selenium; using OpenQA.Selenium.Chrome; @@ -55,21 +54,13 @@ public async Task Init() } _example = app; - var cloudEndpoint = TestHelpers.GetActualRootUrl(TestInitialiser.CloudServerUrl); - if (string.IsNullOrEmpty(cloudEndpoint)) - { - Assert.Inconclusive( - "No external cloud configured. Set CLOUD_ROOT_URL " + - "(e.g. https://cloud.51degrees.com/)."); - return; - } - + // An example that is already running (EXAMPLE_URL) was pointed at + // its cloud and given its key by whoever started it, so neither is + // required here. One this suite launches needs both, and a missing + // one fails this test naming the variable. await _example.StartAsync( - new ExampleAppOptions( - Port: TestHelpers.GetRandomUnusedPort(), - CloudEndpoint: new Uri(cloudEndpoint), - ResourceKey: TestResourceKey.PaidResourceKey, - ExtraEnv: new Dictionary()), + ExampleApps.BuildOptions( + _example, TestHelpers.GetRandomUnusedPort()), CancellationToken.None); // Front the example with a proxy that disables UA client hints, so they diff --git a/ClientSideOverrides/ExampleClientSideOverrideTests.cs b/ClientSideOverrides/ExampleClientSideOverrideTests.cs index 9d96019..d470df1 100644 --- a/ClientSideOverrides/ExampleClientSideOverrideTests.cs +++ b/ClientSideOverrides/ExampleClientSideOverrideTests.cs @@ -5,7 +5,6 @@ using FiftyOne.Pipeline.Cloud.SeleniumTests.Examples; using FiftyOne.Pipeline.Cloud.SeleniumTests.Helpers; using FiftyOne.Pipeline.Cloud.Tests.Common.Helpers; -using FiftyOne.Pipeline.Cloud.Tests.Common.TestElements; using Microsoft.VisualStudio.TestTools.UnitTesting; using OpenQA.Selenium; using OpenQA.Selenium.Chrome; @@ -40,21 +39,13 @@ public async Task Init() } _example = app; - var cloudEndpoint = TestHelpers.GetActualRootUrl(TestInitialiser.CloudServerUrl); - if (string.IsNullOrEmpty(cloudEndpoint)) - { - Assert.Inconclusive( - "No external cloud configured. Set CLOUD_ROOT_URL " + - "(e.g. https://cloud.51degrees.com/)."); - return; - } - + // An example that is already running (EXAMPLE_URL) was pointed at + // its cloud and given its key by whoever started it, so neither is + // required here. One this suite launches needs both, and a missing + // one fails this test naming the variable. await _example.StartAsync( - new ExampleAppOptions( - Port: TestHelpers.GetRandomUnusedPort(), - CloudEndpoint: new Uri(cloudEndpoint), - ResourceKey: TestResourceKey.PaidResourceKey, - ExtraEnv: new Dictionary()), + ExampleApps.BuildOptions( + _example, TestHelpers.GetRandomUnusedPort()), CancellationToken.None); // Front the example with a proxy that disables UA client hints, so they diff --git a/ClientSideOverrides/ExampleRenderTests.cs b/ClientSideOverrides/ExampleRenderTests.cs index e9ae5d7..65d1123 100644 --- a/ClientSideOverrides/ExampleRenderTests.cs +++ b/ClientSideOverrides/ExampleRenderTests.cs @@ -1,12 +1,10 @@ using System; -using System.Collections.Generic; using System.Linq; using System.Threading; using System.Threading.Tasks; using FiftyOne.Pipeline.Cloud.SeleniumTests.Examples; using FiftyOne.Pipeline.Cloud.SeleniumTests.Helpers; using FiftyOne.Pipeline.Cloud.Tests.Common.Helpers; -using FiftyOne.Pipeline.Cloud.Tests.Common.TestElements; using Microsoft.VisualStudio.TestTools.UnitTesting; using OpenQA.Selenium; using OpenQA.Selenium.Chrome; @@ -68,21 +66,13 @@ public async Task Init() } _example = app; - var cloudEndpoint = TestHelpers.GetActualRootUrl(TestInitialiser.CloudServerUrl); - if (string.IsNullOrEmpty(cloudEndpoint)) - { - Assert.Inconclusive( - "No external cloud configured. Set CLOUD_ROOT_URL " + - "(e.g. https://cloud.51degrees.com/)."); - return; - } - + // An example that is already running (EXAMPLE_URL) was pointed at + // its cloud and given its key by whoever started it, so neither is + // required here. One this suite launches needs both, and a missing + // one fails this test naming the variable. await _example.StartAsync( - new ExampleAppOptions( - Port: TestHelpers.GetRandomUnusedPort(), - CloudEndpoint: new Uri(cloudEndpoint), - ResourceKey: TestResourceKey.PaidResourceKey, - ExtraEnv: new Dictionary()), + ExampleApps.BuildOptions( + _example, TestHelpers.GetRandomUnusedPort()), CancellationToken.None); } @@ -122,9 +112,10 @@ public void Example_RendersRealDetectionResult() "example did not render a real detection result server-side"); // The device id is the compact form of the whole result, so check it - // where it is rendered. java and rust do not render one server-side, - // and that is a property of those pages rather than of detection, so - // it is checked where present rather than demanded everywhere. + // where it is rendered. The java example renders no device id + // server-side, and that is a property of that page rather than of + // detection, so the id is checked where present rather than + // demanded everywhere. var deviceId = ValueOfCellAfter(_driver, DeviceIdCellInSameTable); if (deviceId != null) { diff --git a/Examples/ContractSettingsTests.cs b/Examples/ContractSettingsTests.cs new file mode 100644 index 0000000..6f260e8 --- /dev/null +++ b/Examples/ContractSettingsTests.cs @@ -0,0 +1,186 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Runtime.InteropServices; +using FiftyOne.Pipeline.Cloud.Tests.Common; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace FiftyOne.Pipeline.Cloud.SeleniumTests.Examples +{ + /// + /// What the suite demands from the environment, by case. The rules are + /// read through a lookup the test supplies, so nothing here changes the + /// environment of the run around it. + /// + [TestClass] + public class ContractSettingsTests + { + private const string ExampleUrl = "http://localhost:3000/"; + private const string CloudUrl = "https://cloud.51degrees.com/"; + + private static TestConfig Config(params string[] namesAndValues) + { + var values = new Dictionary(); + for (var i = 0; i < namesAndValues.Length; i += 2) + { + values[namesAndValues[i]] = namesAndValues[i + 1]; + } + return new TestConfig( + name => values.TryGetValue(name, out var value) ? value : null); + } + + /// + /// The case an on-premise example in CI is in: the example is already + /// running and there is no cloud and no key. The suite must ask for + /// neither. + /// + [TestMethod] + public void BuildOptions_RunningExample_NeedsNoCloudUrlAndNoKey() + { + var app = new ExternalExampleApp(new Uri(ExampleUrl)); + + var options = ExampleApps.BuildOptions(app, 1234, Config()); + + Assert.AreEqual(1234, options.Port); + Assert.IsNull(options.CloudEndpoint, + "no cloud endpoint should be invented when none is configured"); + Assert.IsNull(options.ResourceKey, + "no resource key should be demanded for a running example"); + } + + /// + /// Set for a running example, both values are still passed on, so a + /// cloud example started by CI is unaffected. + /// + [TestMethod] + public void BuildOptions_RunningExample_PassesOnWhatIsSet() + { + var app = new ExternalExampleApp(new Uri(ExampleUrl)); + + var options = ExampleApps.BuildOptions(app, 1234, Config( + TestConfig.RootUrlVariable, CloudUrl, + TestConfig.PaidResourceKeyVariable, "a-key")); + + Assert.AreEqual(new Uri(CloudUrl), options.CloudEndpoint); + Assert.AreEqual("a-key", options.ResourceKey); + } + + /// + /// An example this suite launches has to be told where the cloud is, + /// so a missing CLOUD_ROOT_URL fails and says which variable to set. + /// + [TestMethod] + public void BuildOptions_LaunchedExample_RequiresTheCloudUrl() + { + var app = new SubprocessExampleApp(ExampleApps.Descriptors["dotnet"]); + + var ex = Assert.ThrowsExactly( + () => ExampleApps.BuildOptions(app, 1234, Config( + TestConfig.PaidResourceKeyVariable, "a-key"))); + + StringAssert.Contains(ex.Message, TestConfig.RootUrlVariable); + } + + /// + /// The same for the key, which the example needs to call the cloud. + /// + [TestMethod] + public void BuildOptions_LaunchedExample_RequiresTheResourceKey() + { + var app = new SubprocessExampleApp(ExampleApps.Descriptors["dotnet"]); + + var ex = Assert.ThrowsExactly( + () => ExampleApps.BuildOptions(app, 1234, Config( + TestConfig.RootUrlVariable, CloudUrl))); + + StringAssert.Contains(ex.Message, TestConfig.PaidResourceKeyVariable); + } + + /// + /// EXAMPLE_URL selects the already-running example whatever + /// EXAMPLE_LANG says. + /// + [TestMethod] + public void TryCreate_ExampleUrlSet_UsesTheRunningExample() + { + var values = new Dictionary + { + ["EXAMPLE_URL"] = ExampleUrl, + ["EXAMPLE_LANG"] = "rust", + }; + + var created = ExampleApps.TryCreate( + name => values.TryGetValue(name, out var value) ? value : null, + out var app, + out var skipReason); + + Assert.IsTrue(created, skipReason); + Assert.IsInstanceOfType(app); + Assert.AreEqual(new Uri(ExampleUrl), app.BaseUrl); + } + + /// + /// A language with no descriptor is skipped rather than failed, and + /// the reason says what was asked for and what is known. + /// + [TestMethod] + public void TryCreate_UnknownLanguage_ExplainsItself() + { + var created = ExampleApps.TryCreate( + name => name == "EXAMPLE_LANG" ? "cobol" : null, + out _, + out var skipReason); + + Assert.IsFalse(created); + StringAssert.Contains(skipReason, "cobol"); + StringAssert.Contains(skipReason, "dotnet"); + } + + /// + /// A missing value is reported by name and the value itself is never + /// part of a message. + /// + [TestMethod] + public void TestConfig_MissingValue_NamesTheVariable() + { + var config = Config(); + + Assert.IsNull(config.Optional(TestConfig.RootUrlVariable)); + var ex = Assert.ThrowsExactly( + () => config.Require(TestConfig.RootUrlVariable)); + StringAssert.Contains(ex.Message, TestConfig.RootUrlVariable); + } + + /// + /// An empty value counts as missing, which is what a CI script that + /// exports an unset variable produces. + /// + [TestMethod] + public void TestConfig_EmptyValue_CountsAsMissing() + { + var config = Config(TestConfig.RootUrlVariable, ""); + + Assert.IsNull(config.Optional(TestConfig.RootUrlVariable)); + } + + /// + /// The python example is launched through the interpreter inside its + /// virtual environment, which is in a different place on Windows. + /// + [TestMethod] + public void VenvPython_IsTheInterpreterForThisOperatingSystem() + { + var python = ExampleApps.VenvPython(".venv"); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.AreEqual( + Path.Combine(".venv", "Scripts", "python.exe"), python); + } + else + { + Assert.AreEqual(Path.Combine(".venv", "bin", "python"), python); + } + } + } +} diff --git a/Examples/ExampleApps.cs b/Examples/ExampleApps.cs index 2b268af..e15091c 100644 --- a/Examples/ExampleApps.cs +++ b/Examples/ExampleApps.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; using System.IO; +using System.Runtime.InteropServices; +using FiftyOne.Pipeline.Cloud.Tests.Common; namespace FiftyOne.Pipeline.Cloud.SeleniumTests.Examples { @@ -45,20 +47,32 @@ public static class ExampleApps /// Returns false when no descriptor is registered for the selected language, /// letting the caller decide whether that should fail the test or skip it. /// - public static bool TryCreate(out IExampleApp app, out string skipReason) + public static bool TryCreate(out IExampleApp app, out string skipReason) => + TryCreate(Environment.GetEnvironmentVariable, out app, out skipReason); + + /// + /// As , reading + /// the variables through so the rules + /// can be tested without changing the process environment. + /// + public static bool TryCreate( + Func getVariable, + out IExampleApp app, + out string skipReason) { - var external = Environment.GetEnvironmentVariable(ExampleUrlVar); + var external = getVariable(ExampleUrlVar); if (!string.IsNullOrEmpty(external)) { app = new ExternalExampleApp(new Uri(external)); skipReason = null; return true; } - if (!Descriptors.TryGetValue(SelectedLang, out var descriptor)) + var lang = getVariable(ExampleLangVar) ?? "dotnet"; + if (!Descriptors.TryGetValue(lang, out var descriptor)) { app = null; skipReason = - $"No example descriptor registered for EXAMPLE_LANG='{SelectedLang}'. " + + $"No example descriptor registered for EXAMPLE_LANG='{lang}'. " + $"Known: {string.Join(", ", Descriptors.Keys)}."; return false; } @@ -67,6 +81,48 @@ public static bool TryCreate(out IExampleApp app, out string skipReason) return true; } + /// + /// Options for starting , read from the + /// environment. + /// + public static ExampleAppOptions BuildOptions(IExampleApp app, int port) => + BuildOptions(app, port, TestConfig.Instance()); + + /// + /// Options for starting . + /// + /// + /// An example that is already running (EXAMPLE_URL) was pointed at + /// its cloud and given its key by whoever started it, and + /// ignores these options. + /// So CLOUD_ROOT_URL and PAID_RESOURCE_KEY are passed on when they + /// are set but are not required, which lets an on-premise example + /// run the Contract tests with neither. An example this suite + /// launches itself needs both, and a missing one throws an + /// naming the variable. + /// + public static ExampleAppOptions BuildOptions( + IExampleApp app, int port, TestConfig config) + { + string rootUrl; + string resourceKey; + if (app is ExternalExampleApp) + { + rootUrl = config.OptionalRootUrl; + resourceKey = config.OptionalPaidResourceKey; + } + else + { + rootUrl = config.RootUrl; + resourceKey = config.PaidResourceKey; + } + return new ExampleAppOptions( + Port: port, + CloudEndpoint: rootUrl == null ? null : new Uri(rootUrl), + ResourceKey: resourceKey, + ExtraEnv: new Dictionary()); + } + /// Per-language launch descriptors. public static readonly IReadOnlyDictionary Descriptors = new Dictionary @@ -144,11 +200,11 @@ public static bool TryCreate(out IExampleApp app, out string skipReason) "device-detection-python", "fiftyone_devicedetection_examples"), // the venv's python (absolute path) - Command: Path.Combine( + Command: VenvPython(Path.Combine( RepoPaths.SiblingsRoot, "device-detection-python", "fiftyone_devicedetection_examples", - ".venv", "bin", "python"), + ".venv")), Args: new[] { "-m", "fiftyone_devicedetection_examples.cloud.gettingstarted_web" }, ReadinessPath: "/", StartupTimeoutSeconds: 60, @@ -159,7 +215,16 @@ public static bool TryCreate(out IExampleApp app, out string skipReason) ["resource_key"] = o.ResourceKey, }, BuildCommand: "bash", - BuildArgs: new[] { "-c", "python3 -m venv .venv && .venv/bin/pip install -e ." }, + // a Windows python install may have no working "python3", + // so fall back to "python" to create the venv, then use + // the venv's own interpreter, whose path differs by OS + BuildArgs: new[] + { + "-c", + "(python3 -m venv .venv || python -m venv .venv) && " + + VenvPython(".venv").Replace(Path.DirectorySeparatorChar, '/') + + " -m pip install -e .", + }, BuildTimeoutSeconds: 600, JsonEndpointPath: "/json"), ["php"] = new ExampleDescriptor( @@ -223,5 +288,15 @@ public static bool TryCreate(out IExampleApp app, out string skipReason) }, BuildTimeoutSeconds: 900), }; + + /// + /// Path of the python interpreter inside the virtual environment at + /// . A Windows virtual environment keeps it in + /// Scripts/python.exe, everywhere else it is bin/python. + /// + public static string VenvPython(string venv) => + RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? Path.Combine(venv, "Scripts", "python.exe") + : Path.Combine(venv, "bin", "python"); } } diff --git a/TestInitialiser.cs b/TestInitialiser.cs index e2295f1..48da74e 100644 --- a/TestInitialiser.cs +++ b/TestInitialiser.cs @@ -1,26 +1,24 @@ using FiftyOne.Pipeline.Cloud.Tests.Common; -using Microsoft.VisualStudio.TestTools.UnitTesting; namespace FiftyOne.Pipeline.Cloud.SeleniumTests { /// - /// Assembly-level setup. The cloud is always external; its URL comes from - /// the test configuration (TEST_CONFIG_FILE -> cloud_root_url). + /// Where the tests find the cloud under test. /// - [TestClass] - public class TestInitialiser + /// + /// There is no assembly-level setup. The cloud URL used to be read once + /// for the whole run, which failed every test, including Contract tests + /// against an example that was already running, whenever CLOUD_ROOT_URL + /// was unset. It is now read by the tests that talk to the cloud + /// directly, and only they fail when it is missing. + /// + public static class TestInitialiser { /// - /// Base URL of the cloud under test. Empty when no external cloud is - /// configured. + /// Base URL of the cloud under test, from CLOUD_ROOT_URL. Throws an + /// naming the variable + /// when it is not set. /// - public static string CloudServerUrl; - - /// Reads the external cloud URL from the test configuration. - [AssemblyInitialize] - public static void TestInitialise(TestContext testContext) - { - CloudServerUrl = TestConfig.Instance().RootUrl; - } + public static string CloudServerUrl => TestConfig.Instance().RootUrl; } } diff --git a/TestsCommon/Helpers/TestHelpers.cs b/TestsCommon/Helpers/TestHelpers.cs index 9ebe0e9..a4ab1b7 100644 --- a/TestsCommon/Helpers/TestHelpers.cs +++ b/TestsCommon/Helpers/TestHelpers.cs @@ -166,22 +166,12 @@ public static ServerListener StartGeoServer(string pageData, CancellationToken t } /// - /// Get the actual Root Url used for testing. - /// If test environment is configured to use URL from configuration - /// file, then return the URL specified from configuration file. Else - /// return the AspNet Test Server URL being passed in. + /// The cloud URL to test against: CLOUD_ROOT_URL when it is set, + /// otherwise the URL passed in. /// public static string GetActualRootUrl(string testServerRootUrl) { - var config = TestConfig.Instance(); - if (String.IsNullOrEmpty(config.RootUrl)) - { - return testServerRootUrl; - } - else - { - return config.RootUrl; - } + return TestConfig.Instance().OptionalRootUrl ?? testServerRootUrl; } } } diff --git a/TestsCommon/TestConfig.cs b/TestsCommon/TestConfig.cs index b80ae70..15f5763 100644 --- a/TestsCommon/TestConfig.cs +++ b/TestsCommon/TestConfig.cs @@ -4,26 +4,45 @@ namespace FiftyOne.Pipeline.Cloud.Tests.Common { // Test environment configuration. Every value comes from an environment // variable so nothing sensitive is committed - this repository is public. + // + // Nothing is read until a test asks for it, so a missing variable only + // fails the tests that need it. public class TestConfig { + // Names of the environment variables. + public const string RootUrlVariable = "CLOUD_ROOT_URL"; + public const string FreeResourceKeyVariable = "FREE_RESOURCE_KEY"; + public const string PaidResourceKeyVariable = "PAID_RESOURCE_KEY"; + public const string EnterpriseV4LicenseVariable = "ENTERPRISE_V4_LICENSE"; + // Base URL of the cloud under test. - public string RootUrl => Require(_rootUrl); + public string RootUrl => Require(RootUrlVariable); // Free resource key (no extra properties). - public string FreeResourceKey => Require(_freeResourceKey); + public string FreeResourceKey => Require(FreeResourceKeyVariable); // Paid resource key. - public string PaidResourceKey => Require(_paidResourceKey); - // Enterprise V4 license, passed to the JS endpoint to unlock paid properties. - public string EnterpriseV4License => Require(_enterpriseV4License); + public string PaidResourceKey => Require(PaidResourceKeyVariable); + // Enterprise V4 license, passed to the JS endpoint to unlock paid + // properties. + public string EnterpriseV4License => Require(EnterpriseV4LicenseVariable); - private const string _rootUrl = "CLOUD_ROOT_URL"; - private const string _freeResourceKey = "FREE_RESOURCE_KEY"; - private const string _paidResourceKey = "PAID_RESOURCE_KEY"; - private const string _enterpriseV4License = "ENTERPRISE_V4_LICENSE"; + // Base URL of the cloud under test, or null when it is not set. + public string OptionalRootUrl => Optional(RootUrlVariable); + // Paid resource key, or null when it is not set. + public string OptionalPaidResourceKey => Optional(PaidResourceKeyVariable); private static readonly object _syncLock = new object(); private static TestConfig _instance; - private TestConfig() { } + private readonly Func _getVariable; + + // Reads from the given lookup instead of the process environment, so + // the rules can be tested without changing the environment of the + // whole test run. + public TestConfig(Func getVariable) + { + _getVariable = getVariable + ?? throw new ArgumentNullException(nameof(getVariable)); + } public static TestConfig Instance() { @@ -31,22 +50,26 @@ public static TestConfig Instance() { lock (_syncLock) { - _instance ??= new TestConfig(); + _instance ??= new TestConfig( + Environment.GetEnvironmentVariable); } } return _instance; } - // Reads a required variable; fails naming the variable if it is missing. - private static string Require(string name) + // Reads a variable, returning null when it is missing or empty. + public string Optional(string name) { - string value = Environment.GetEnvironmentVariable(name); - if (string.IsNullOrEmpty(value)) - { - throw new InvalidOperationException( - $"Required environment variable '{name}' is not set."); - } - return value; + var value = _getVariable(name); + return string.IsNullOrEmpty(value) ? null : value; + } + + // Reads a required variable and fails naming the variable if it is + // missing. The value itself is never part of the message. + public string Require(string name) + { + return Optional(name) ?? throw new InvalidOperationException( + $"Required environment variable '{name}' is not set."); } } } From 4295350fc749c7877dab7931d444929a9ea807e4 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 17 Sep 2026 00:28:11 +0100 Subject: [PATCH 2/4] TEST: Print the reason for every skipped test A test that cannot run says why and reports itself as skipped. None of those reasons reached the console, because the console logger that dotnet test uses prints the name of a skipped test and nothing else, so a CI log showed a row of skips with no way to tell a missing example from a stale page. Output written inside a test does not help either, as it goes to the test host and never reaches the console. A logger runs in the process that writes the console output, so this adds a small one that prints the reason alongside each skip. It is a separate assembly because the test platform only looks for a logger in one whose name ends with ".TestLogger", and it is turned on by test.runsettings, which the test project points at. A plain "dotnet test" gets it with no extra arguments, so no caller has to change. --- Helpers/SkipReasonLoggerTests.cs | 84 +++++++++++++ SeleniumApiTests.csproj | 10 ++ .../FiftyOne.SeleniumTests.TestLogger.csproj | 13 +++ TestLogger/SkipReasonLogger.cs | 110 ++++++++++++++++++ test.runsettings | 15 +++ 5 files changed, 232 insertions(+) create mode 100644 Helpers/SkipReasonLoggerTests.cs create mode 100644 TestLogger/FiftyOne.SeleniumTests.TestLogger.csproj create mode 100644 TestLogger/SkipReasonLogger.cs create mode 100644 test.runsettings diff --git a/Helpers/SkipReasonLoggerTests.cs b/Helpers/SkipReasonLoggerTests.cs new file mode 100644 index 0000000..3fd70f7 --- /dev/null +++ b/Helpers/SkipReasonLoggerTests.cs @@ -0,0 +1,84 @@ +using System; +using FiftyOne.SeleniumTests.TestLogger; +using Microsoft.VisualStudio.TestPlatform.ObjectModel; +using PlatformTestResult = + Microsoft.VisualStudio.TestPlatform.ObjectModel.TestResult; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace FiftyOne.Pipeline.Cloud.SeleniumTests.Helpers +{ + /// What the skip reason logger prints. + [TestClass] + public class SkipReasonLoggerTests + { + private static PlatformTestResult Result( + TestOutcome outcome, string message) + { + var testCase = new TestCase( + "Suite.Class.TheTest", + new Uri("executor://mstest/v2"), + "SeleniumApiTests.dll") + { + DisplayName = "TheTest", + }; + return new PlatformTestResult(testCase) + { + Outcome = outcome, + ErrorMessage = message, + }; + } + + /// A skip prints its test and its reason on one line. + [TestMethod] + public void Format_Skipped_PrintsTheReason() + { + var line = SkipReasonLogger.Format(Result( + TestOutcome.Skipped, + "Assert.Inconclusive failed. No example descriptor " + + "registered for EXAMPLE_LANG='cobol'.")); + + Assert.AreEqual( + " Skipped TheTest, because: No example descriptor " + + "registered for EXAMPLE_LANG='cobol'.", + line); + } + + /// A passing test prints nothing. + [TestMethod] + public void Format_Passed_PrintsNothing() + { + Assert.IsNull(SkipReasonLogger.Format( + Result(TestOutcome.Passed, "some output"))); + } + + /// A skip with no reason prints nothing. + [TestMethod] + public void Format_SkippedWithNoReason_PrintsNothing() + { + Assert.IsNull(SkipReasonLogger.Format( + Result(TestOutcome.Skipped, " "))); + } + + /// + /// A reason raised in a class or test initialiser arrives wrapped by + /// the test framework and spread over lines. It still reads as one + /// line naming the cause. + /// + [TestMethod] + public void Format_ReasonFromAnInitialiser_ReadsAsOneLine() + { + var line = SkipReasonLogger.Format(Result( + TestOutcome.Skipped, + "Class Initialization method Some.Class.ClassInit threw " + + "exception.\r\nMicrosoft.VisualStudio.TestTools." + + "UnitTesting.AssertInconclusiveException: " + + "Assert.Inconclusive failed. Required environment variable " + + "'CLOUD_ROOT_URL' is not set..")); + + StringAssert.Contains(line, "'CLOUD_ROOT_URL' is not set."); + Assert.IsFalse(line.Contains("AssertInconclusiveException"), + "the exception type is noise, not a reason"); + Assert.IsFalse(line.Contains('\n'), "one skip should be one line"); + } + } +} diff --git a/SeleniumApiTests.csproj b/SeleniumApiTests.csproj index c12e1fd..e36a626 100644 --- a/SeleniumApiTests.csproj +++ b/SeleniumApiTests.csproj @@ -4,8 +4,18 @@ net10.0 false x64;ARM64 + $(MSBuildProjectDirectory)/test.runsettings + + + + + + + diff --git a/TestLogger/FiftyOne.SeleniumTests.TestLogger.csproj b/TestLogger/FiftyOne.SeleniumTests.TestLogger.csproj new file mode 100644 index 0000000..d1d86f6 --- /dev/null +++ b/TestLogger/FiftyOne.SeleniumTests.TestLogger.csproj @@ -0,0 +1,13 @@ + + + + net10.0 + false + FiftyOne.SeleniumTests.TestLogger + + + + + + + diff --git a/TestLogger/SkipReasonLogger.cs b/TestLogger/SkipReasonLogger.cs new file mode 100644 index 0000000..2c3839c --- /dev/null +++ b/TestLogger/SkipReasonLogger.cs @@ -0,0 +1,110 @@ +using System; +using Microsoft.VisualStudio.TestPlatform.ObjectModel; +using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client; +using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; + +namespace FiftyOne.SeleniumTests.TestLogger +{ + /// + /// Prints the reason for every skipped test to the console. + /// + /// + /// + /// The console logger that dotnet test uses prints the name of a + /// skipped test and nothing else, so a run that skips everything reads as + /// a success and the reason is only visible by re-running with + /// --logger "console;verbosity=detailed". A test that writes the + /// reason itself does not help, as output written inside a test goes to + /// the test host and never reaches the console. + /// + /// + /// A logger runs in the same process as the console output, so it can + /// print. This one is discovered because the assembly name ends with + /// ".TestLogger" and it is built next to the tests, and it is turned on by + /// test.runsettings, which the test project points at. Nothing has to be + /// added to the dotnet test command line. + /// + /// + [FriendlyName(FriendlyNameValue)] + [ExtensionUri(ExtensionUriValue)] + public class SkipReasonLogger : ITestLogger + { + /// Name used to enable this logger. + public const string FriendlyNameValue = "skipreasons"; + + /// Unique identifier of this logger. + public const string ExtensionUriValue = + "logger://51degrees/selenium/skipreasons/v1"; + + // Noise the assertion and the test framework add around the reason. + private static readonly string[] _prefixesToTrim = + { + "Assert.Inconclusive failed. ", + "Microsoft.VisualStudio.TestTools.UnitTesting." + + "AssertInconclusiveException: ", + }; + + /// Subscribes to the results of the run. + public void Initialize(TestLoggerEvents events, string testRunDirectory) + { + if (events == null) + { + throw new ArgumentNullException(nameof(events)); + } + events.TestResult += (sender, e) => + { + var line = Format(e?.Result); + if (line != null) + { + Console.WriteLine(line); + } + }; + } + + /// + /// The line to print for a result, or null when the result is not a + /// skip or carries no reason. + /// + public static string Format(TestResult result) + { + if (result == null + || result.Outcome != TestOutcome.Skipped + || string.IsNullOrWhiteSpace(result.ErrorMessage)) + { + return null; + } + return $" Skipped {result.TestCase?.DisplayName}, because: " + + Tidy(result.ErrorMessage); + } + + /// + /// Removes the wrapping the assertion adds, so the reason the test + /// gave is what the reader sees. Line breaks become spaces, keeping + /// one skip to one line. + /// + public static string Tidy(string message) + { + if (message == null) + { + return null; + } + var tidied = message.Replace("\r\n", " ").Replace('\n', ' ').Trim(); + bool trimmed; + do + { + trimmed = false; + foreach (var prefix in _prefixesToTrim) + { + var at = tidied.IndexOf(prefix, StringComparison.Ordinal); + if (at >= 0) + { + tidied = tidied.Remove(at, prefix.Length); + trimmed = true; + } + } + } + while (trimmed); + return tidied; + } + } +} diff --git a/test.runsettings b/test.runsettings new file mode 100644 index 0000000..f118068 --- /dev/null +++ b/test.runsettings @@ -0,0 +1,15 @@ + + + + + + + + + From 8a142122ced682fb5c9fdb2196ac7be6e6263432 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 17 Sep 2026 00:28:17 +0100 Subject: [PATCH 3/4] DOC: Say which settings each category needs, and check this suite here The README said a missing variable only fails the test that reads it, which was not true while the cloud URL was read for the whole run. It now says which category needs which variable, that a run against an example that is already running needs no cloud URL and no key, and how to read the reason for a skipped test. The repository had no CI of its own, so a change could only be tried by a language repository checking out main. The "Build and test" workflow builds the suite and runs the tests that need no browser, no cloud and no keys. It sets no environment variables, which is the point, as the suite must not demand settings the tests being run do not use. --- .github/workflows/build-and-test.yml | 31 ++++++++++++++++ README.md | 54 +++++++++++++++++++++++----- 2 files changed, 76 insertions(+), 9 deletions(-) create mode 100644 .github/workflows/build-and-test.yml diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml new file mode 100644 index 0000000..279c4fd --- /dev/null +++ b/.github/workflows/build-and-test.yml @@ -0,0 +1,31 @@ +# Builds the suite and runs the tests that need no browser, no cloud and no +# keys, so a change here is checked before a language repository picks it up. +name: Build and test + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +jobs: + build-and-test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-dotnet@v4 + with: + dotnet-version: '10.0.x' + + - name: Build + run: dotnet build -c Release + + # No environment variables are set, which is the point: the suite must + # not demand settings that the tests being run do not use. + - name: Test + run: > + dotnet test -c Release --no-build + --filter "TestCategory!=Contract&TestCategory!=CloudInternal" diff --git a/README.md b/README.md index d13262d..2b76964 100644 --- a/README.md +++ b/README.md @@ -21,29 +21,51 @@ Select a subset with `--filter TestCategory=Contract` or ## How a run is wired -- **Example app** — for CI the example is launched by the caller and its URL is +- **Example app** - for CI the example is launched by the caller and its URL is passed in `EXAMPLE_URL`. For local runs set `EXAMPLE_LANG` (e.g. `dotnet`) and the suite launches the example from the sibling checkout. -- **Cloud endpoint** — `CLOUD_ROOT_URL` is the cloud the example talks to and the - cloud the `CloudInternal` tests hit directly. -- **Browser** — by default a local Chrome driver is used. Set `SELENIUM_URL` to use +- **Cloud endpoint** - `CLOUD_ROOT_URL` is the cloud an example launched here is + pointed at, and the cloud the `CloudInternal` tests hit directly. An example + that is already running was pointed at its data by whoever started it, so with + `EXAMPLE_URL` set the suite needs neither `CLOUD_ROOT_URL` nor + `PAID_RESOURCE_KEY`, which is what lets an on-premise example run `Contract`. +- **Browser** - by default a local Chrome driver is used. Set `SELENIUM_URL` to use a Selenium grid instead (CI uses a standalone grid). ## Configuration -All configuration is read from environment variables — nothing is read from a file, +All configuration is read from environment variables - nothing is read from a file, and no keys are committed. | Variable | Used by | Notes | |---|---|---| -| `CLOUD_ROOT_URL` | all | Base cloud URL, e.g. `https://cloud.51degrees.com/`. | -| `PAID_RESOURCE_KEY` | all | Resource key used by the tests. | +| `CLOUD_ROOT_URL` | `CloudInternal`, and `Contract` when the suite launches the example | Base cloud URL, e.g. `https://cloud.51degrees.com/`. | +| `PAID_RESOURCE_KEY` | `CloudInternal`, and `Contract` when the suite launches the example | Resource key used by the tests. | | `FREE_RESOURCE_KEY` | `CloudInternal` | Free resource key for the JS-endpoint tests. | | `ENTERPRISE_V4_LICENSE` | `CloudInternal` | License passed to the JS endpoint to unlock paid properties. | -| `SELENIUM_URL` | optional | Selenium grid URL; omit for a local Chrome driver. | +| `SELENIUM_URL` | optional | Selenium grid URL, omit for a local Chrome driver. | | `EXAMPLE_URL` / `EXAMPLE_LANG` | `Contract` | The example app to test (CI / local). | -A missing variable only fails the test that reads it. +A missing variable only fails the tests that read it, and the failure names the +variable. Nothing is read for the run as a whole, so a `Contract` run against a +running example (`EXAMPLE_URL`) needs no cloud URL and no key, and there is no +need to invent placeholder values to get a run started. + +## Why a test was skipped + +A test that cannot run says why and is reported as skipped rather than failed, +for example an example app that renders no device id, or a language with no +descriptor. `dotnet test` prints the name of a skipped test and nothing else, +which reads in a CI log as though everything is fine, so this repository ships +a small logger that prints the reason as well: + +``` + Skipped Example_RendersRealDetectionResult, because: ... No example descriptor + registered for EXAMPLE_LANG='bogus'. Known: dotnet, java, node, python, php, rust. +``` + +It is in `TestLogger` and it is turned on by `test.runsettings`, which the test +project points at, so a plain `dotnet test` gets it with no extra arguments. ## Running locally @@ -56,6 +78,14 @@ export EXAMPLE_LANG="dotnet" dotnet test --filter TestCategory=Contract ``` +Contract against an example that is already running, which is how CI calls it +and the only settings it needs: + +```bash +export EXAMPLE_URL="http://localhost:8080/" +dotnet test --filter TestCategory=Contract +``` + CloudInternal against a cloud you control: ```bash @@ -66,6 +96,12 @@ export ENTERPRISE_V4_LICENSE="" dotnet test --filter TestCategory=CloudInternal ``` +## This repository's own CI + +The "Build and test" workflow builds the suite and runs the tests that need no +browser, no cloud and no keys, on every push and pull request. It is what +proves a change here before any language repository picks it up. + ## CI integration - **Cloud CI** checks this repo out as `../selenium-api-tests`, builds it once, runs From a1b4f82c02a004c4ad16243535c20a513dc24f91 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 17 Sep 2026 07:02:18 +0100 Subject: [PATCH 4/4] DOC: Replace the em dashes in two comments Three em dashes were left in comments in this public repository, two in ExampleRenderTests and one in ExternalExampleApp, the class the new BuildOptions comment points at. House style is no em dash anywhere, so they are replaced with commas and a linking word. Comment text only, and the suite still builds clean. --- ClientSideOverrides/ExampleRenderTests.cs | 4 ++-- Examples/ExternalExampleApp.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ClientSideOverrides/ExampleRenderTests.cs b/ClientSideOverrides/ExampleRenderTests.cs index 65d1123..f950c4a 100644 --- a/ClientSideOverrides/ExampleRenderTests.cs +++ b/ClientSideOverrides/ExampleRenderTests.cs @@ -14,8 +14,8 @@ namespace FiftyOne.Pipeline.Cloud.SeleniumTests.ClientSideOverrides { /// - /// Loads a real example app the way an ordinary visitor would — plain - /// desktop Chrome, no mobile emulation and no header-injecting proxy — and + /// Loads a real example app the way an ordinary visitor would, with plain + /// desktop Chrome, no mobile emulation and no header-injecting proxy, and /// checks that the page the example renders server-side actually contains a /// real device-detection result. This is the "does the example work at all" /// counterpart to , which only diff --git a/Examples/ExternalExampleApp.cs b/Examples/ExternalExampleApp.cs index a61e466..42ac68d 100644 --- a/Examples/ExternalExampleApp.cs +++ b/Examples/ExternalExampleApp.cs @@ -17,7 +17,7 @@ public sealed class ExternalExampleApp : IExampleApp public ExternalExampleApp(Uri baseUrl) => BaseUrl = baseUrl; /// - /// No-op — the example is already running. Whoever started it must point it at + /// No-op, as the example is already running. Whoever started it must point it at /// the test's cloud and resource key; the options here are not applied. /// public Task StartAsync(ExampleAppOptions options, CancellationToken ct) => Task.CompletedTask;