Skip to content

Run the Selenium contract tests against the on-premise web example, and fail the job when they fail - #349

Merged
Automation51D merged 4 commits into
mainfrom
feature/onpremise-web-selenium
Sep 18, 2026
Merged

Automation51D merged 4 commits into
mainfrom
feature/onpremise-web-selenium

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

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 it

  1. listened on port 5001 whatever the environment said (app.py, run()),
  2. only found its data file through config.json, which names the Lite file, and the Lite file has neither DeviceType nor the JavaScript override properties the contract tests need,
  3. inlined the client-side script in the page, with no /51Degrees.core.js route, 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: 9 for the contract tests against the cloud example (the four SessionStorageCache_* cases, "no json refresh call is expected on the second page").

Why failures were reported as success

common-ci runs ci/run-integration-tests.ps1 through steps/run-script.ps1, which dot-sources it (. $Script @Parameters). The script ended with dotnet test ... and never looked at its exit code. A non-zero $LASTEXITCODE does not stop PowerShell, and the job's result is the $LASTEXITCODE left 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 (a throw) reaches the job. The pytest integration results are still caught, but only because they are written as XML and the publish step has action_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 pwsh wrapper). A script that ends with a failing native command, or with exit 1, gives a job exit code of 0. A script that throws gives 1.

What changed

  • onpremise/gettingstarted_web/app.py
    • The port comes from PORT (default still 5001).
    • A data file path in 51DEGREES_DD_PATH replaces the one in config.json. When the variable is not set, config.json and the file search work as before. A path that does not exist raises an error naming the variable.
    • A /51Degrees.core.js route serves the JavaScriptBuilder output, copied from the cloud example.
  • onpremise/gettingstarted_web/config.json adds the SequenceElement. build_from_configuration only builds the elements it is given, and without that element the script is rendered with var sequence=;, which does not parse, so the browser never defines fod. 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.html loads /51Degrees.core.js instead of inlining the script, and renders the device id row, as the cloud page does.
  • example_utils.py adds DATA_FILE_ENV_VAR and get_data_file_path().
  • tests/test_onpremisegettingstartedweb.py covers the new route, the sequence number in the script, the script reference in the page, and both environment variable cases.
  • ci/run-integration-tests.ps1
    • Sets up the examples' virtual environment once, then runs the contract tests against the cloud example on port 8097 and the on-premise example on port 8098. The on-premise example uses assets/TAC-HashV41.hash, which ci/fetch-assets.ps1 fetches when the device detection licence is given. Without it, the on-premise run is skipped with a warning.
    • Waits for each example with a polling loop (up to 180 seconds, stopping early if the example exits), because the old curl --retry-connrefused wait 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.
    • Writes each contract run as test-results/integration/selenium-<cloud|onpremise>.trx, so the results are published with the other integration results.
    • Collects failures (pytest exit code and each contract run) and throws at the end if there are any, so the job goes red.

The JSON path is /json for both examples, which is what the suite already expects for EXAMPLE_LANG=python, so the suite needs no change.

Verification

  • Locally, with Flask and fiftyone_pipeline_core only (no native engine), build_config() picks up the environment variable, reports a missing file by the variable's name, falls back to config.json when the variable is not set, the app registers /, /51Degrees.core.js and /json, and run() passes the port from PORT. The template parses with Jinja2. The polling loop and Remove-Job stopping the example were checked with pwsh 7.5.
  • utm-lint.ps1 -Campaign device-detection-python is clean.
  • Locally, a pipeline built from the new config.json (the fiftyone_pipeline_core elements only) renders a script that passes node --check, and the same pipeline without the SequenceElement renders one that fails it.
  • CI, the "Pull Requests" workflow dispatched on this branch with dryrun=true, so nothing is merged.
    • Run 35152825906 (first commit). Cloud 9 of 9 passed. On-premise 1 passed, 8 failed (the missing SequenceElement). The job threw "Integration tests failed" and went red, which shows a Selenium failure now fails the job.
    • Run 35154616442 (with the fix). All 15 build and test jobs succeeded, and every one printed Total tests: 9, Passed: 9 for the cloud example and again for the on-premise example.

Outstanding

  • The four SessionStorageCache_* failures in the nightly of 16 September ran against fiftyone_pipeline_core 4.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.
  • The same dot-sourcing problem applies to ci/run-unit-tests.ps1, which ends with exit $LASTEXITCODE. A unit test failure is still caught there, but only through the published XML results, not through the exit code.
  • Neither web example ends with a "Find out more" section. I left that alone here.
  • This stays a draft so the nightly does not merge it.

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.js route instead of inlining it. Neither depends on the
other and either can go first.

The root cause was checked again away from CI. A pipeline built from the
new config.json, with the fiftyone_pipeline_core elements only, renders
var sequence=1; and passes node --check. The same pipeline with the
SequenceElement taken out renders var sequence=; and node --check
reports SyntaxError: Unexpected token ';'.

What a reviewer still has to judge

  • The Selenium runs themselves are only proven in CI. Run
    35154616442
    is on the head commit of this branch, 81a18fec, and all 15 build and
    test jobs passed.
  • $LASTEXITCODE after ./python/run-integration-tests.ps1 is the code of
    whatever native command ran last inside that script, so the first entry
    in $failures is a good signal rather than a contract. The pytest
    results are still caught properly through the published XML, which the
    common-ci publish_test_results action reads with action_fail: true.
  • The device id row added to the on-premise page carries a comment saying
    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 81a18fe in
run 35154616442,
a dispatch of the Pull Requests workflow with dryrun set, so it built the
branch 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 cloud
example 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.

…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.
@jwrosewell
jwrosewell marked this pull request as ready for review September 17, 2026 20:07
@Automation51D
Automation51D merged commit 53f38c7 into main Sep 18, 2026
20 checks passed
@Automation51D
Automation51D deleted the feature/onpremise-web-selenium branch September 18, 2026 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants