Run the Selenium contract tests against the on-premise web example, and fail the job when they fail - #349
Merged
Conversation
…ple, and fail the job when they fail The on-premise getting started web example now takes its port from PORT, its data file from 51DEGREES_DD_PATH (config.json is still used when the variable is not set), serves the client-side script from /51Degrees.core.js and renders the device id, matching the cloud example. ci/run-integration-tests.ps1 now runs the contract tests against both the cloud example (port 8097) and the on-premise example (port 8098, against the TAC data file), writes each result as a trx file with the other integration results, and throws when any run fails. The script is dot-sourced by common-ci, so the exit code of dotnet test was lost once the performance step ran its own commands, which is why four failing contract tests were reported as success.
…ation, so its script parses Without it the client-side script is rendered as 'var sequence=;', which does not parse, so 'fod' is never defined. The first CI run of the contract tests against the on-premise example failed 8 of 9 for this reason.
…be reverted)" This reverts commit 8ba11b5.
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.
Problem
The shared Selenium contract tests in selenium-api-tests (category
Contract) ran in this repository's CI against the cloud web example only. The on-premise getting started web example could not be driven the same way, because itapp.py,run()),config.json, which names the Lite file, and the Lite file has neitherDeviceTypenor the JavaScript override properties the contract tests need,/51Degrees.core.jsroute, unlike the cloud example.Separately, a Selenium failure did not fail the job. The nightly run 35047684743 reported success on all 30 build and test jobs, and every one of them printed
Failed! - Failed: 4, Passed: 5, Skipped: 0, Total: 9for the contract tests against the cloud example (the fourSessionStorageCache_*cases, "no json refresh call is expected on the second page").Why failures were reported as success
common-ci runs
ci/run-integration-tests.ps1throughsteps/run-script.ps1, which dot-sources it (. $Script @Parameters). The script ended withdotnet test ...and never looked at its exit code. A non-zero$LASTEXITCODEdoes not stop PowerShell, and the job's result is the$LASTEXITCODEleft when the whole step ends, so the performance step that runs next reset it to 0 with its own commands. Only an error that stops the script (athrow) reaches the job. The pytest integration results are still caught, but only because they are written as XML and the publish step hasaction_fail: true. The Selenium results were written nowhere, so nothing caught them.I reproduced this with pwsh 7.5 using the same shape (a dot-sourced script, a later native command, and the GitHub
pwshwrapper). A script that ends with a failing native command, or withexit 1, gives a job exit code of 0. A script that throws gives 1.What changed
onpremise/gettingstarted_web/app.pyPORT(default still 5001).51DEGREES_DD_PATHreplaces the one inconfig.json. When the variable is not set,config.jsonand the file search work as before. A path that does not exist raises an error naming the variable./51Degrees.core.jsroute serves the JavaScriptBuilder output, copied from the cloud example.onpremise/gettingstarted_web/config.jsonadds theSequenceElement.build_from_configurationonly builds the elements it is given, and without that element the script is rendered withvar sequence=;, which does not parse, so the browser never definesfod. The first CI run on this branch failed 8 of 9 contract tests against the on-premise example for this reason. The inlined script had the same fault before this change.onpremise/gettingstarted_web/templates/index.htmlloads/51Degrees.core.jsinstead of inlining the script, and renders the device id row, as the cloud page does.example_utils.pyaddsDATA_FILE_ENV_VARandget_data_file_path().tests/test_onpremisegettingstartedweb.pycovers the new route, the sequence number in the script, the script reference in the page, and both environment variable cases.ci/run-integration-tests.ps1assets/TAC-HashV41.hash, whichci/fetch-assets.ps1fetches when the device detection licence is given. Without it, the on-premise run is skipped with a warning.curl --retry-connrefusedwait gave up at once in the nightly log ("Failed to connect to localhost port 8097 after 0 ms") and the tests only passed because the example happened to be up by then.test-results/integration/selenium-<cloud|onpremise>.trx, so the results are published with the other integration results.The JSON path is
/jsonfor both examples, which is what the suite already expects forEXAMPLE_LANG=python, so the suite needs no change.Verification
fiftyone_pipeline_coreonly (no native engine),build_config()picks up the environment variable, reports a missing file by the variable's name, falls back toconfig.jsonwhen the variable is not set, the app registers/,/51Degrees.core.jsand/json, andrun()passes the port fromPORT. The template parses with Jinja2. The polling loop andRemove-Jobstopping the example were checked with pwsh 7.5.utm-lint.ps1 -Campaign device-detection-pythonis clean.config.json(thefiftyone_pipeline_coreelements only) renders a script that passesnode --check, and the same pipeline without theSequenceElementrenders one that fails it.dryrun=true, so nothing is merged.SequenceElement). The job threw "Integration tests failed" and went red, which shows a Selenium failure now fails the job.Total tests: 9, Passed: 9for the cloud example and again for the on-premise example.Outstanding
SessionStorageCache_*failures in the nightly of 16 September ran againstfiftyone_pipeline_core4.5.25. Version 4.5.26 was published to PyPI later that day, the runs above installed it, and the cloud example passed all 9 in both. I did not change anything for this.ci/run-unit-tests.ps1, which ends withexit $LASTEXITCODE. A unit test failure is still caught there, but only through the published XML results, not through the exit code.Sibling change
The same fault and the same fix are in the PHP on-premise repository, in
device-detection-php-onpremise#271.
Both examples built their client-side script without a
SequenceElement,so both rendered
var sequence=;, and both now serve the script from a/51Degrees.core.jsroute instead of inlining it. Neither depends on theother and either can go first.
The root cause was checked again away from CI. A pipeline built from the
new
config.json, with thefiftyone_pipeline_coreelements only, rendersvar sequence=1;and passesnode --check. The same pipeline with theSequenceElementtaken out rendersvar sequence=;andnode --checkreports
SyntaxError: Unexpected token ';'.What a reviewer still has to judge
35154616442
is on the head commit of this branch,
81a18fec, and all 15 build andtest jobs passed.
$LASTEXITCODEafter./python/run-integration-tests.ps1is the code ofwhatever native command ran last inside that script, so the first entry
in
$failuresis a good signal rather than a contract. The pytestresults are still caught properly through the published XML, which the
common-ci
publish_test_resultsaction reads withaction_fail: true.the device id is "the compact form of everything else in this table".
The screen width and height in that table come from the client side and
are not part of the device id, so the sentence is looser than it should
be. It is a comment in a template and nothing reads it.
Taken out of draft on 17 September 2026
Green on the current head
81a18feinrun 35154616442,
a dispatch of the Pull Requests workflow with
dryrunset, so it built thebranch and merged nothing. All 15 build and test jobs passed, being Ubuntu,
Windows and macOS against Python 3.10 to 3.14.
The point of this change is that the on-premise Selenium run is real and
fails the job when it fails, so the job logs were read rather than the
conclusion. Every one of the 15 jobs prints
"Running Selenium tests against the onpremise example on port 8098" and
then
Total tests: 9, Passed: 9, alongside the same nine against the cloudexample on port 8097. So the contract tests ran against the on-premise
example on every job and none of them was skipped.
The branch is level with
main, so nothing needed merging in.The three points under "What a reviewer still has to judge" above stand.
None of them stops this merging, being a comment in a template, an exit
code used as a signal rather than a contract, and the Selenium runs being
proven in CI rather than locally.