Repository navigation
Conversation
… the framework A staleness audit (grepping every doc for v1 flag names, the old command name and stale counts) found what my earlier passes had missed: README.md - The seam section still documented the v1 arguments as current: a table of --ocrPath/--ocrSimilarity/--ocrMaxSim/--ocrWaitTime/--ocrMaxCount/--ocrAction/ --ocrDetail/--ocrPSM/--ocrOEM, and examples driving findTargetImage. Replaced with the canonical table and examples, pointing at docs/CONTRACT.md as the source of truth so the two can no longer drift. - The illustrative run output still showed 'findTargetImage --imagePath=...'; it now shows what the suite actually prints. - The feature table and the coverage-honesty notes still named --textHint, --maxSim, --imageAction, --imageMaxCount, --ocrPath, --ocrSimilarity and --ocrDetail. - The section intro claimed the interface was findTargetImage; it is autobdd, with findTargetImage as the deprecated alias. The duplicated 'Basic usage' block is gone (subsumed by 'The front door' + 'Examples'). docs/CONTRACT.md - The v1 argument table was left dangling under a 'Raw arguments' heading, which read as if it were current. Retitled 'Raw argument reference (v1 - deprecated)' with a pointer to the §2b mapping, and the two prose mentions of --imageAction/--ocrDetail and the 'center' row now use the canonical names. seam + suite - The help text's known-gap line and the suite's user-facing known-gap line said --ocrDetail=word; they now say --box, which is what the reader would type. - The help feature now asserts the canonical flag appears AND that the deprecated v1 names are listed, so the deprecation table cannot silently disappear. framework/libs/screen_session.js - This was the last v1 emitter: it still built 'findTargetImage --imagePath=... --imageAction=...', so after the rename every framework call would warn on stderr seven times. Now emits the front door with the canonical flags. textHint was a REGEX and --match-text is literal by default, so --match-regex is passed to preserve the existing semantics exactly, twice documented in the code. Verified: the generated framework command matches with 'clicked' dispatched and an EMPTY stderr (no deprecation warnings); the base suite runs 131 passed, 0 failed. The framework e2e suite itself was not run - it needs the L2 image with Chrome - so that consumer is verified at the command-string level plus the engine's legacy/v2 coverage. No v1 flag names remain in any documentation except the deprecation tables and the intentional historical notes in CHANGELOG.md.
The product splits into two lines. v3.0.0 is untouched - tag, release and image
stay exactly as shipped, as the last release of the FRAMEWORK line (base + Chrome +
WebdriverIO + Cucumber, published as xyteam/autobdd-framework:3.0.0 and the
xyteam/autobdd alias). 4.0.0 is cut from it and narrows the product to the
substrate: a docker-runnable GUI (Xvfb + openbox + x11vnc) with the Oculix screen
engine behind one CLI, so higher-level tools - a framework, a script, an agent -
drive it. No browser, no runner, no BDD layer.
- .docker/autobdd-base.dockerfile: new ARG AUTOBDD_VERSION=4.0.0, recorded as
version= in /etc/autobdd-versions. The framework image already selects its base
with the same arg (FROM xyteam/autobdd-base:${AUTOBDD_VERSION}), so the two lines
stay coherent. Also fixed a stale stamp field: oculix= pointed at
/opt/autobdd/third_party/xysikulixapi/lib, which stopped existing when the seam
became in-tree, so the field was silently empty.
- seam: --version now reports the recorded version alongside the build stamp, the
Oculix jar and Node, and names the product ('AutoBDD base image').
- package.json + package-lock.json: 3.0.0 -> 4.0.0.
- README: repositions the product as the 4.0.0 base line with the goal stated
plainly (a GUI you drive from higher-level tools), documents the two lines and
which image belongs to which, and adds a v4.0.0 row to the version table with
v3.0.0 marked frozen.
- CHANGELOG: a v4.0.0 section describing the re-scope and what it meant in practice
(one interface, a readable vocabulary, discoverability without starting the
engine, self-contained natives, a conformance suite that doubles as the spec).
- suite: gates on the release - provenance asserts version=4.0.0 in
/etc/autobdd-versions and the version feature asserts --version reports it.
Group I's discovery features now invoke the surface under test (TARGET_BIN) rather
than a hardcoded findTargetImage, so the log matches the banner's stated surface.
Verified: autobdd --version -> 'version: 4.0.0'; make base-test -> 133 passed,
0 failed, ALL FEATURES OK; one.sh I -> 18 passed.
…roperly
CI failed on the previous commit, correctly:
- provenance: records the product version (missing 'version=4.0.0' in: version=dev
- version: reports the version (missing 'version: 4.0.0' in: version: dev
The gate I wrote was the wrong shape: it asserted a hardcoded literal, but the image
records the version it was BUILT as, and CI builds with --build-arg AUTOBDD_VERSION=dev.
'dev' is not a lie -- AUTOBDD_VER is simultaneously the image tag and the base tag the
framework image pins via FROM, so a pull-request build legitimately stamps dev.
Fix, in two parts:
suite (features.sh)
- provenance now asserts the stamp records a non-empty version (not 'unknown'), and
additionally that it equals EXPECTED_VERSION when the caller declares one. Unset, the
release gate is dormant and the run is a consistency check.
- version now asserts --version agrees with the image's own stamp, and equals
EXPECTED_VERSION when declared. The suite therefore works unchanged for both a dev
build and a release build, instead of hardcoding a number that will rot.
workflow (conformance.yml)
- AUTOBDD_VER is resolved from package.json in a first step (GITHUB_ENV) rather than
being a literal 'dev': package.json is the single source of truth, and using one value
for the build arg, the image tags and the suite's expectation is what makes a version
drift fail the build instead of shipping.
- the base-test step exports AutoBDD_Ver (the tag this workflow built, since the test
projects' .env defaults to dev) and EXPECTED_VERSION; the framework step exports
AutoBDD_Ver too.
Docs: the test project's README documents EXPECTED_VERSION; CHANGELOG records the wiring.
Verified all three cases, not just the happy one:
1. built as dev, no expectation -> 30 passed, 0 failed, 'consistency checked, no
release gate' printed
2. built as dev, expecting 4.0.0 -> FAILS ('got dev, want 4.0.0'), exit 1 -- so the
gate bites and the failure propagates
3. built as 4.0.0, expecting 4.0.0 -> 30 passed, 0 failed
group I: '--version agrees with the image's own stamp = 4.0.0' and 'is the declared
release = 4.0.0'
HARDENING, NOT A PROVEN FIX -- read this before treating it as one. The intermittent exit-status defect recorded twice in this project (rc=139, SIGSEGV, on a call that had already written a correct result) is NOT reproduced by this change, and I could not reproduce it at all to diagnose it: 20 sequential calls and 24 calls at 6-way concurrency inside a container, 44 runs, every one rc=0 with empty stderr. Both historical sightings happened on an abnormally loaded host (three conformance suites and zombie containers plus a build running at once), and in both cases the crash output was never captured, so there is no artifact to work from. What this does remove is the one place the engine races a native thread against process teardown, and it sits on what is now the default automation path: Region.highlight() paints on a background native thread and returns immediately, and with --flash=0s the engine painted and then slept 0 ms -- exiting while that painter may still be running. --flash=0 means 'no flash', so painting was pointless work as well as a race. The helper now returns before painting when the pause is zero. Nothing visible is lost: the caller asked for no flash. Verified: the flash feature still measures the default path (default 2128 ms vs --flash=0s 1131 ms -> delta 997 ms, bounded), and the full suite runs 135 passed, 0 failed with EXPECTED_VERSION=4.0.0 set. If rc=139 recurs, capture stderr and the payload together -- the run line already records [rc=N] -- and the next candidate is the JVM/native teardown itself (skipping it on the success path), which cannot be chosen responsibly without that evidence.
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
1 participant
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.
Hardening, not a proven fix — please read the first section before merging
I could not reproduce the intermittent
rc=139(SIGSEGV on a call that had already written a correct result), so this change is not offered as its fix.What I tried
rc=0, stderr emptyrc=0, stderr emptyThe two historical sightings both happened on an abnormally loaded host — three conformance suites plus zombie containers plus a build at once — and in neither case was the crash output captured. There is no artifact to work from, and any specific "fix" would be a guess.
What this does remove
One real race, on what is now the default automation path.
Region.highlight()paints on a background native thread and returns immediately; with--flash=0sthe engine painted and then slept 0 ms, i.e. it exited while that painter might still be running.--flash=0means no flash, so the paint was pointless work as well as a race. The helper now returns before painting when the pause is zero.Nothing visible is lost: the caller asked for no flash.
Verified
The default flash path still paints and pauses (997 ms measured), so the feature it exists for is unchanged; only the
0scase stops painting.If it recurs
The run line already records
[rc=N], so capture stderr and the payload together and there will finally be something to diagnose. The next candidate is the JVM/native teardown on the success path (skipping it rather than running it), which cannot be chosen responsibly without that evidence — masking a non-zero exit in the dispatcher would hide genuine crashes, so I have deliberately not done that.