Skip to content

Build the failure to match example's pipeline on every run - #74

Merged
Automation51D merged 1 commit into
mainfrom
fix/example-pipeline-cache-and-key
Sep 18, 2026
Merged

Automation51D merged 1 commit into
mainfrom
fix/example-pipeline-cache-and-key

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

The problem

examples/cloud/failureToMatch.php built its pipeline once and wrote it to
a file, then reused that file on every later run. A pipeline carries the
properties of the resource key it was built with, so a run with a key
entitled to IsMobile still ended in:

PHP Fatal error:  Uncaught Exception: Property 'ismobile' not found in data
for element 'device'. This is because your resource key does not include
access to this property. Properties that are included for this key under
'device' are ...

listing the properties of whichever key had run first. Read on
17 September 2026 by running the example with one key and then with
another.

The file name was built as __DIR__ . "failure_to_match_pipeline.pipeline"
with no separator, so the file landed beside the folder the example lives
in rather than inside it.

What changed

The example builds its pipeline on every run, so the answer always belongs
to the key the example was given. The comment now says what a site does
instead, which is to keep the pipeline and throw it away when the resource
key changes, rather than showing a cache with no way to invalidate it.

How it was checked

tests/ExampleTests.php gains a test that runs the example as its own
process with the resource key in its environment, checks it answers, and
checks it leaves no serialized pipeline in or beside the examples folder.

Without the change the test fails, naming the file:

-Array &0 ()
+Array &0 (
+    0 => '...\tests/../examples/cloudfailure_to_match_pipeline.pipeline'
+)
FAILURES! Tests: 1, Assertions: 3, Failures: 1.

With the change:

OK (1 test, 3 assertions)

The rest of the suites were run against https://cloud.51degrees.com with
a resource key entitled to the properties the examples read. The Unit
suite passes, 5 tests. The Integration suite passes apart from the test
that reads tests/51Degrees.csv, which the build fetches and a local
checkout does not have, and three tests that only run on Linux.

Sibling change

The Python repository has a fault in the same example, although a
different one, in
device-detection-python#350.
There the example read the resource key from the older environment
variable name alone, so a reader who had set the current name was told to
create a key. Neither change depends on the other.

CI

The "Pull Requests" workflow was dispatched on this branch with
dryrun=true. Run
35163144670
passed on all 10 build and test jobs, and it ran on the head commit,
914e9182.

The same fault is still in the getting started web example

examples/cloud/classes/GettingStartedWeb.php line 116 builds its file
name the same way, __DIR__ . 'gettingStartedWeb.pipeline' with no
separator, and reuses whatever it wrote on the next run. So that example
has both faults this change fixes in the failure to match example. It is
left alone here because it is a different example with a web test of its
own, and it needs its own change. *.pipeline is in .gitignore, so
nothing stale was ever committed.

What a reviewer still has to judge

The new test checks that the example answers and leaves no serialized
pipeline behind. It does not look for the fault markers the other example
tests look for, so an example that printed its first line and then a fatal
error would still pass. runScript also reads standard output to the end
before reading standard error, which would deadlock if an example ever
wrote more to standard error than the pipe holds. Neither matters for what
this example prints.

Taken out of draft on 17 September 2026

Re-run on the current head 914e918 as
run 35190505104,
a dispatch of the Pull Requests workflow with dryrun set. All 14 jobs
passed, being Ubuntu and macOS against PHP 7.4, 8.0, 8.1, 8.2 and 8.3. The
job logs show the Unit suite at OK (5 tests, 22 assertions) and the
Integration suite at 35 tests, which is OK (35 tests, 649 assertions) on
Ubuntu and Tests: 35, Assertions: 597, Skipped: 3 on macOS, the three
skips being the tests that only run on Linux.

The branch is level with main, so nothing needed merging in.

The two points under "What a reviewer still has to judge" above stand, and
the getting started web example still carries the same fault, which is left
for its own change.

The example kept the pipeline it built in a file and reused it on the
next run, so a run with one resource key answered with the properties of
whichever key had written the file. Running the example with a key
entitled to IsMobile still ended in 'Property ismobile not found in data
for element device', listing the properties of the earlier key. The file
name was also built without a separator, so it landed beside the examples
folder rather than in it.

The example now builds its pipeline each run and the comment says what a
site does instead, which is to keep the pipeline and throw it away when
the resource key changes.

tests/ExampleTests.php runs the example as its own process, checks it
answers and checks it leaves no serialized pipeline behind. The test
fails before the change, naming the file left in examples/, and passes
after it.
@jwrosewell

Copy link
Copy Markdown
Contributor Author

Proven in CI

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

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 14 jobs finished green, being 10 build and test jobs (PHP 7.4, 8.0, 8.1, 8.2 and 8.3 on both Ubuntu and macOS), the pull request lookup, the configure step, the performance comparison and the completion step.

Reading the job logs rather than the conclusions, all five Ubuntu jobs report "OK (35 tests, 649 assertions)" and all five macOS jobs report "Tests: 35, Assertions: 597, Skipped: 3", the three skipped being the ones macOS does not run. Every job also reports "OK (5 tests, 22 assertions)" for the second suite. Searching the whole log finds no "FAILURES!", no "ERRORS!" and no non-zero failure count.

The macOS jobs waited a long time for a runner this morning, so the run took longer than the others. That was queueing for runners and not anything in the branch.

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:08
@Automation51D
Automation51D merged commit dbf5273 into main Sep 18, 2026
29 checks passed
@Automation51D
Automation51D deleted the fix/example-pipeline-cache-and-key branch September 18, 2026 05:29
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