Ask only for the settings a test uses, and print skip reasons - #10
Merged
Merged
Conversation
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.
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.
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.
Contributor
Author
|
The "Build and test" workflow added in this branch has run on this pull request and passed, with no environment variables set at all: "Build succeeded" and "Passed! - Failed: 0, Passed: 24, Skipped: 0, Total: 24" (run 35162545567, ubuntu-latest, .NET 10). That is the first change proved in real CI rather than on one machine, because on main the same command with nothing set aborts the whole run on the missing CLOUD_ROOT_URL. |
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.
jwrosewell
marked this pull request as ready for review
September 17, 2026 20:00
This was referenced Sep 17, 2026
Merged
oleksandrlazarenko-pi
added a commit
that referenced
this pull request
Sep 18, 2026
Brings in #10 (contract settings, skip reasons) and #11 (ARM64 Linux browser drivers). Three conflicts, all where both sides added: - TestsCommon/TestConfig.cs: main made every variable read through the lookup a TestConfig is given. The demo settings are added on top, and RequireFirst now reads through Optional too, so a configuration built for a test no longer ignores its lookup for the demo's resource key. Browser51Did/DemoSettingsTests.cs covers it. - Examples/ExampleApps.cs: main's BuildOptions and the demo registry are independent, and both are kept. - README.md: both new category rows are kept.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Three problems reported by the language repositories that run
dotnet test --filter TestCategory=Contractagainst their web examples.1. The suite demanded settings it did not use
TestInitialiserreadCLOUD_ROOT_URLin[AssemblyInitialize], so an unsetvariable aborted the whole run whatever the tests being run needed. The
Contracttests then readPAID_RESOURCE_KEYas well, inBrowserCache/SessionStorageCacheTests.csand in theTestInitializemethodsof the three classes under
ClientSideOverrides. An example that is alreadyrunning was pointed at its data and given its key by whoever started it, and
ExternalExampleApp.StartAsyncignores both values, so an on-premise examplehad nothing to give and the callers were setting placeholders to get a run
started.
Now nothing is read for the run as a whole. The cloud URL is read where it is
used, and the options for starting an example are built in one place,
ExampleApps.BuildOptions:that test naming the variable,
EXAMPLE_URL) needs neither and ispassed whatever happens to be set,
CloudInternal, which talks to the cloud directly, still needs them andstill fails naming the variable.
Examples/ContractSettingsTests.cscovers those rules by case. They readthrough a lookup the test supplies, so they do not change the environment of
the run around them.
README.mdsaid "A missing variable only fails the test that reads it", whichwas not true. It now says which category needs which variable, and that a run
against a running example needs no cloud URL and no key.
2. Skip reasons never reached the console
A test that cannot run says why and reports itself as skipped, but the console
logger
dotnet testuses prints the name of a skipped test and nothing else.A CI log therefore showed a row of skips with no way to tell a missing example
from a stale page, and the reason was only visible by re-running with
--logger "console;verbosity=detailed".Writing the reason from inside the test does not work, because output written
in a test goes to the test host and never reaches the console. Two other
routes were tried and failed as well, being the console logger verbosity set
in a run settings file, which
dotnet testignores because it adds its ownconsole logger, and writing to the raw standard output handle, which goes
nowhere for the same reason as the test output.
A logger runs in the process that writes the console output, so this adds a
small one in
TestLogger. It is a separate assembly because the test platformonly looks for a logger in one whose name ends with ".TestLogger", and it is
enabled by
test.runsettings, which the test project points at throughRunSettingsFilePath. A plaindotnet testpicks it up with no extraarguments, so no caller has to change how it calls the suite.
3. The comment about the rust example was out of date
ClientSideOverrides/ExampleRenderTests.cssaid java and rust render nodevice id server-side. The rust example renders one, in the same table as the
device type (
examples/device-detection-examples/src/web_support/mod.rsinthe rust repository lists
("Device Id", "deviceid")among the properties itsserver-side table renders). Only java is named now.
No rust-specific skip depended on the comment. The check reads the device id
where it is rendered and skips the assertion where the page has no such row,
which applies to every language, and it is unchanged. It was left conditional
rather than demanded for everything except java, because the same tests run
against on-premise examples too and those pages were not all checked.
Also
environment, which is
Scripts/python.exeon Windows andbin/pythonelsewhere. The descriptor assumed
bin/python, which does not exist onWindows. Creating the environment now falls back from
python3topython,as a Windows install may have no working
python3.a language repository checking out
main. The "Build and test" workflowbuilds the suite and runs the tests that need no browser, no cloud and no
keys, on every push and pull request. It sets no environment variables,
which is the point of the first change.
Evidence
The Selenium tests themselves were not run, as the machine used has no browser
and must not start one. Everything below is from real runs of the tests that
need no browser.
Before, on
main, with no environment variables set:After, on this branch, same command and still no variables set:
A
Contracttest against an example that is already running, with no cloud URLand no key. On
mainit aborts on the missing variable. On this branch it getsall the way to the browser step and fails only because there is no grid on the
machine it was run on, which is what proves the settings are no longer in the
way:
A
CloudInternaltest with nothing set still fails, clearly and on its own:An example this suite launches still needs both values and says which one is
missing:
Skip reasons, from a plain
dotnet testwith no logger arguments:On
mainthe same case, run with placeholder values set as the CI scripts doso that the run starts at all, prints the nine names and no reasons at all:
The "Build and test" workflow added here runs on this pull request, so the
checks on it are the same evidence in real CI rather than on one machine.
What a reviewer should decide
for
CLOUD_ROOT_URLandPAID_RESOURCE_KEYwhen they run an on-premiseexample. Nothing outside this repository was changed here, and a placeholder
still works, it is simply no longer needed.
rather than checked where present. That needs someone to confirm that every
on-premise example page renders one.
Related
51Degrees/common-ci#242 makes a nightly step fail when the repository script
it ran failed. Until that is in, a failing run of this suite leaves the job
green, so the skip reasons added here are printed into a log nobody is sent
to look at. The two do not depend on each other and can be merged in either
order.