Skip to content

TEST: Let a property the client script populates have no value - #590

Merged
Automation51D merged 1 commit into
mainfrom
test/value-types-without-a-value
Sep 18, 2026
Merged

Automation51D merged 1 commit into
mainfrom
test/value-types-without-a-value

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

What is wrong

ValueTests.valueTypes, in both copies, walks every available property of the
engine and calls getValue() on it to check the type. A property the 51Degrees
client script populates has no value on a request that carries no client script
evidence, and the requests these tests make carry none, so getValue() throws
and the test errors.

Against https://cloud.51degrees.com with a resource key entitled to
HasWebDriver, device-detection.cloud reports:

ValueCloudTests.ValueTests_Cloud_ValueTypes
HasWebDriver has no value because the 51Degrees JavaScript that populates it
has not run on this request. The data holds 'Unknown', which is neither True
nor False.
Tests run: 5, Failures: 0, Errors: 1, Skipped: 0

The test only passes today because the keys it usually runs with are not
entitled to those properties, so the failure is waiting for whichever key gains
one. The same shape of failure is what closed issue
#584, where the
data file gained IsVisible, IsVisibleJavaScript, HasWebDriver,
HasWebDriverJavaScript and IsHeadless and the on-premise tests broke.

The change

Both copies of the test, the cloud one in
device-detection.cloud/src/test/java/fiftyone/devicedetection/cloud/ValueTests.java
and the shared one in
device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/data/ValueTests.java,
now read hasValue() first. Where there is no value the test checks that the
reason was given, which is what a caller reads, and moves on. Where there is a
value the type check is unchanged. Nothing is skipped silently.

How it was checked

mvn -pl device-detection.cloud -am test -Dtest=ValueCloudTests,MissingPropertyHandlingTests
with TestResourceKey set to a key carrying HasWebDriver, on 17 September
2026:

  • before the change, Tests run: 7, Failures: 0, Errors: 1, Skipped: 0, the
    error being ValueTests_Cloud_ValueTypes as quoted above;
  • after the change, Tests run: 7, Failures: 0, Errors: 0, Skipped: 0.

The on-premise copy is not exercised locally because the data file is not
available on this machine, so the CI run on this branch is what covers it.

The same change in the other repositories

ip-intelligence-java#178
makes the same change to that repository's ValueTests.valueTypes, where the
same walk over every listed property hits a property the resource key is not
entitled to.

CI on this branch, 17 September 2026

The "Pull Requests" workflow was dispatched on this branch with dryrun=true,
which builds the branch and merges nothing. Run
35190312090
ran on b0c0884, the head of this branch, and concluded success with all 20
jobs green. In the Ubuntu_Java_17 job the Maven totals are 100 tests run, 0
failures, 0 errors and 5 skipped, with no error lines in the log.

This branch is one commit behind main, that commit being the automated package
update #591, and
it merges cleanly.
The nightly checks out the pull request, merges main into it and builds
that before it merges anything, so the combination is tested there.

The value type tests asked every available property for its value and its
type. A property the 51Degrees client script populates, HasWebDriver for
one, has no value at all on a request that carries no client script
evidence, and these requests carry none, so asking for the type throws and
the test fails with a key entitled to those properties. Both copies of the
test now read whether there is a value first, check that the reason was
given when there is none, and check the type when there is one.

Checked against the live cloud with a key carrying HasWebDriver. Before
the change ValueCloudTests reports one error, HasWebDriver has no value
because the 51Degrees JavaScript that populates it has not run on this
request, and after it the five tests pass.
@jwrosewell

Copy link
Copy Markdown
Contributor Author

Proven in CI, with one finding that is not about this branch

Full build and test run on this branch: https://github.com/51Degrees/device-detection-java/actions/runs/35190312090

A draft pull request only runs the link lint, so the Pull Requests workflow was dispatched against this branch as a dry run to get real evidence. Nothing was merged, and the log shows why: with no pull request based on this branch the run builds the branch on its own as pull request 0, and the checkout and completion steps both print "Not running for a PR".

All 20 jobs finished green, being 16 build and test jobs (Java 8, 11, 17 and 21 on Ubuntu and Windows, Java 8, 11, 17 and 21 on macOS x64, Java 11, 17 and 21 on macOS ARM, and Java 17 on Ubuntu ARM), the pull request lookup, the configure step, the performance comparison and the completion step.

Reading the job logs rather than the conclusions, each job reports 9 Maven module summaries adding up to 100 tests run with 0 failures, 0 errors and 0 skipped, the same on every platform and every Java version.

The finding

Searching the logs for failure text rather than trusting the green ticks turns up one line that the job did not fail on:

Failed!  - Failed:     9, Passed:     0, Skipped:     0, Total:     9, Duration: 155 ms - SeleniumApiTests.dll (net10.0)

That is the Ubuntu ARM Java 17 job, and it is the only one of the 16. The other 15 jobs all run the same suite and report "Passed! - Failed: 0, Passed: 9" taking between 32 seconds and 1 minute 25. The ARM job fails all nine in 155 milliseconds, and every one of them carries the same cause:

An error occurred trying to start process
'.../selenium-api-tests/bin/Release/net10.0/runtimes/linux/native/selenium-manager'
... Exec format error

"Exec format error" means the file is a binary for another architecture, so this is the x64 Selenium Manager being launched on an ARM64 runner. It is a gap in what the Selenium package ships for linux-arm64 rather than anything in this branch, and no browser was ever started. It is worth raising separately, because the step does not fail the job, so the run looks green whilst nine tests are failing on that one platform.

Nothing here touches what this pull request changes. The pull request has not been marked ready and no review has been requested.

@jwrosewell
jwrosewell marked this pull request as ready for review September 17, 2026 20:14
@Automation51D
Automation51D merged commit 7969f2d into main Sep 18, 2026
41 checks passed
@Automation51D
Automation51D deleted the test/value-types-without-a-value branch September 18, 2026 04:03
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