Skip to content

Check what the umbrella getting started examples answer, and test a data file reload - #108

Merged
jwrosewell merged 5 commits into
rcfrom
examples/umbrella-and-reload-tests
Nov 14, 2026
Merged

jwrosewell merged 5 commits into
rcfrom
examples/umbrella-and-reload-tests

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Two test gaps, with no package code to change.

  1. The umbrella getting started tests check nothing (DX-node-36). device-detection/fiftyone.devicedetection/examples/gettingstarted.test.js lines 51 and 64 to 71 at 758fea81 evaluates the example's code and ends the test after four seconds whatever it printed, and ip-intelligence/fiftyone.ipintelligence/examples/gettingstarted.test.js lines 27 to 30 only requires the module, so its answer is printed, if at all, after the test has ended. This was left under "What remains" in #95.
  2. No Node.js test reloads an on-premise engine's data file (DX-node-19, from #55 "Tests"). A search of both on-premise packages' tests for refreshData or reload finds nothing. The C library pins what a reload must do for an engine built with no Property names, in HashPositionsReloadTests.NothingNamed_SmallThenLarge (test/dd/hash/HashPositionsTests.cpp line 530 at 51degrees-cxx 7c4ac436). There the engine keeps the Property list it started with and reads each of those Properties from the reloaded file, where they sit at other positions.

What the specification says

device-detection-specification/required-examples.md lines 14 to 23 at c8128ca4:

it is likely to be sufficient for a test to show that the example runs and completes without failure.

Those tests MUST include a run of each example with no arguments or settings, as someone would run it from a fresh clone, so that the run uses the free data files.

What changed

  1. Both umbrella tests now run the example in its own process, from its own folder, as a person runs it, and the example must end with status 0. With a resource key set, the Device Detection one must answer true for the iPhone User-Agent and false for the Windows one, and the IP Intelligence one must name a country for 8.8.8.8. The country depends on the cloud service's data, which today answers "Pakistan" where the example's comment expects "United States", so the test only asks for a name rather than a message saying why there is none. With no key each must say how to create one. CI always sets a key, so the runs below take the first branch.
  2. A new device-detection/fiftyone.devicedetection.onpremise/tests/reloadProperties.test.js builds an engine with no Property names on a copy of the free Lite data file, records its Property list and its IsMobile, PlatformName and BrowserName for an iPhone User-Agent, replaces the copy with the Enterprise data file CI fetches, refreshes the data file as the update service does, and checks that the engine now holds the Enterprise data set, keeps the same Property list and gives the same three values.

IP Intelligence has no pair of free data files with the same Properties (the ASN file has Asn and AsnName and the Lite file has the registration Properties), so its reload stays covered by the C library's IpiPositionsReloadTests.

Before this test was written, a throwaway run printed what a reload does to each engine (run 43559640564). The Device Detection engine on the Lite file, reloaded onto the Enterprise file, had 284 Properties in its metadata and still 15 available, as on the Lite file. The IP Intelligence engine on the ASN file, reloaded onto the Lite file, had 3 with none available, as neither of its two is in the Lite file. Both match the C library's rule.

Tests

Each ran on one Ubuntu runner with Node.js 24, with the native modules built, the data files fetched as CI fetches them and the CI resource key, in a throwaway workflow that is not part of this draft.

  • Red, run 43563068117, on rc with the Device Detection umbrella example answering the opposite of what it detects and the IP Intelligence one printing an empty line for the country. The old tests passed 2 of 2, and the new ones then failed 2 of 2, with "Expected: "true" Received: "false"" and an empty line where a country name was expected.

  • Green, run 43563058024 at 24c65ee6, the same as this head but for the wording of a comment and an unused require the lint refused, where both umbrella tests passed. The reload test ran there and failed, 1 failed and 2 passed, because the distributor did not serve the free Lite data file at the time ("The free data file (lite-hash-v41) is not available at present"). The reload test passed in run 43560740891 at 43d2d0c3. That same run failed the IP Intelligence umbrella test, 1 failed and 2 passed, with the country then fixed as "United States" and the cloud service answering "Pakistan", which is why the test now asks only for a name. 481b5fc later changed the reload test to build one Pipeline with no file system watcher, as below, so the head's reload test is proven by the full build.

The first two full builds of this draft failed. Run 43563687773 failed the lint on an unused require of path in both umbrella tests, which 423be61 removes. Run 43569318892 passed on Linux and failed on Windows after the tests had run, because the reload test's Pipeline registered the engine with an update service whose file system watcher fired on the copied file and raised EPERM once the test removed its folder. 481b5fc7 builds one Pipeline with fileSystemWatcher: false, as the test refreshes the file itself.

The reload test passes on rc too, as nothing in the engine changes here. It is a check that a later change to the engine or the C library cannot break the reload without a test failing, and the red above covers only the umbrella tests.

Full build

Run 43573142555 on tmp/pr-matrix-108-c, started by tools/pr-matrix.py without macOS. All 7 Setting 'PullRequestSha' lines give this head, 481b5fc, and Configure printed "Left out 2 macOS entries". Each of the six Build and Test jobs, Ubuntu, Ubuntu ARM and Windows on Node.js 22 and 24, passed 1,389 unit tests with 2 skipped and 145 integration tests with 4 skipped, the reload test and both umbrella tests among them.

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

What remains

  • The IP Intelligence umbrella example's comment gives "United States" as the expected output, which the cloud service no longer answers for 8.8.8.8. The comment is in the example, which #103 also changes, so it is left for after that merges.
  • How a cloud example behaves with no resource key is a rule question still open, so the no-key branch checks only the message the examples print today.

@jwrosewell
jwrosewell marked this pull request as ready for review November 14, 2026 09:25
@jwrosewell
jwrosewell merged commit 21b54ca into rc Nov 14, 2026
2 checks passed
@jwrosewell
jwrosewell deleted the examples/umbrella-and-reload-tests branch November 14, 2026 09:25
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.

1 participant