test: start the behavioral safety net with a fixture-capture tool - #327
Draft
trekin-geoalert wants to merge 5 commits into
Draft
test: start the behavioral safety net with a fixture-capture tool#327trekin-geoalert wants to merge 5 commits into
trekin-geoalert wants to merge 5 commits into
Conversation
added 5 commits
August 12, 2026 04:24
Phase B of the 3.7.0 refactoring. These tests exist to answer one question with evidence rather than assertion: did the refactoring change behaviour? So they may only name surfaces that outlive it - the plugin entry point, widget object names from the .ui, the HTTP conversation, QgsProject layers, QgsSettings keys. A behavioral test that needs editing during Phase C has failed at its job, because the edit is where a behaviour change hides. Feasibility was the open question and it is answered: Mapflow(iface) constructs and initGui() runs inside the test container. The single accommodation is that iface.mainWindow() must return a real QMainWindow - it becomes the parent of the main dialog and of the plugin QObject, and PyQt rejects a MagicMock where it expects a QWidget. That is why test_plugin_boots exists at all: a plugin that fails to construct does not degrade, it silently never appears in QGIS, and it is the precondition for every later journey. The fixtures are captured from the live API rather than written from the spec. Hand-written payloads contain only the fields we already knew about, so they cannot catch a parser breaking on a field the server actually sends - which is exactly the regression class this net is for. capture_fixtures.py discovers every id from an earlier response (list projects, take an id, fetch that project) so it works against any account and hardcodes nothing. Everything it sends is read-only except one opt-in call behind --include-create-failure, which is shaped to be refused and prompts before running. Creating a processing spends real credits, so that could not be the default. The scrubber is tested rather than trusted, because it is the only thing between a real account and a committed fixture. Consistency is the property that is easy to miss: a project id must scrub to the same value in the list response and the detail response, or the two fixtures cannot be used together in one test. Over-scrubbing is a failure too - a fixture that has lost its statuses and model names is no longer a realistic payload - so that is asserted as well. detect-secrets flagged the invented credentials in those tests, correctly by its own rules. The fix is a pragma on the literals plus a constant for the expected output, since `out["password"] == "redacted"` reads as a hardcoded credential to the scanner and there is no reason to teach it otherwise. Placed under tests/qgis/ rather than in a new tests/behavioral/ tier on purpose. A fourth tier means editing the Makefile, and the Makefile is a watched file: any local edit blocks every agent-make invocation until it reaches master, not just dev. That would stall the whole refactor behind a human merge to the release branch. A subdirectory is collected by the existing invocation, and it is the honest classification anyway - these need the QGIS runtime, which is what the tiers actually split on. ## Manual test New behaviour - the capture tool, which is run by hand: - export MAPFLOW_TOKEN with the same base64 token the plugin stores, then run python3 tests/qgis/behavioral/capture_fixtures.py - expect JSON under tests/qgis/behavioral/responses/, one file per endpoint, each with the request that produced it; the run prints what it saved, what it skipped and why, and a count of what was scrubbed - read the files before committing them: the scrubber removes what it recognises, and a payload shape it has not seen is passed through unchanged - an account with no projects, no templates or no mosaics should skip those captures with a stated reason rather than failing Regression surface: none. Nothing under mapflow/ is touched; this adds test files only.
…y key name
Answers two review questions, and fixes three defects the first real capture exposed - one
of them a live credential.
A real Mapbox access token reached a local commit. The scrubber stripped query strings only
from values held under a list of known key names (url, downloadUrl, previewUrl, ...), and
/user/status returns the Mapbox tile URL under webPreviewUrl, which was not on that list.
detect-secrets did not flag it; GitHub push protection rejected the push. Nothing reached
the remote.
The fix is to stop keying on the field name at all. Any value that looks like a URL now has
its query inspected, and parameters whose name suggests a credential are replaced while the
rest survive - so the fixture keeps a realistic tile template with {z}/{x}/{y} and its
format and quality params, and loses only the token. A key allowlist was always going to go
stale on the next field the backend adds; this cannot.
The capture tool stays in the repo, and the size caps move into it. Fixtures rot when the
API changes, and a fixture set nobody can regenerate dies the first time the backend adds a
required field - hand-editing fifteen JSON files is not a maintenance plan. Keeping the tool
only helps if a re-capture reproduces the same small fixtures, so trimming cannot be a
manual pass afterwards. --retrim re-applies the caps, and the URL stripping, to files
already on disk. 609 KB -> 136 KB.
--retrim does not re-scrub ids or emails. Removing a credential-bearing query parameter
twice is a no-op, but remapping already-anonymised ids would break the property that makes
the fixtures usable together: a project id matching across the list and the detail response.
Truncation is preference-aware rather than "keep the first N". A blind cut dropped
🏠 Buildings - Config.DEFAULT_MODEL - and kept a multi-model entry instead, leaving the
processing and result-loading journeys unable to exercise the paths they exist for. Models
that styles.py has a .qml for are kept first, then the list is filled.
Two more defects the capture surfaced. Both cost fixtures came back 400: the endpoint keys
on wdId, a workflow-def id, not the model name the request sent, so the "happy path" fixture
recorded the same rejection as the deliberate one. And every captured project was empty -
sorting by UPDATED DESC surfaces the most recently touched projects, which on a working
account are throwaway test projects. Selection now reads processingCounts and templatesCount
and picks a project that holds work, falls back to another project to find a FAILED
processing, and walks up to ten mosaics to find one containing an image. The request asks
for 100 projects and still saves 3 - which also required save() to stop mutating the payload
it hands back to the caller.
spec/007 records which half of the test suite is temporary, because the intuition runs
backwards: the behavioral suite is permanent - it is the only coverage of user-visible
behaviour in a repo whose UI tier is an empty harness, and the next release is the QGIS 4 /
Qt 6 port. What shrinks is the 43-file Class.__new__(Class) suite.
## Manual test
Re-run the capture - the committed fixtures were taken with the broken cost body and an
empty project, so they need replacing:
python3 tests/qgis/behavioral/capture_fixtures.py
- processing_cost.json should be HTTP 200 with a real price; processing_cost_rejected stays 400
- processings_page.json, templates_by_project.json and mosaic_images.json should be
non-empty, and processing_detail_failed.json should appear if the account ever had a
processing fail
- anything missing is printed under "skipped" with a reason
- grep the output for access_token, signature and secret: the only hits should read
"=scrubbed"
- check 🏠 Buildings survives in workflowDefs everywhere it appears
Regression surface: none. Nothing under mapflow/ is touched.
The first behavioral journey, and the harness the rest will reuse. Logging in is asserted the way a user experiences it: type a token, press Log in, and check what goes over the wire and what appears on screen. No plugin method is named, so Phase C can move all of it. The fake replaces QgsNetworkAccessManager rather than Http. Http moves to infra/ in Phase D, so a test patching it would break during the refactoring it exists to guard; the QGIS class cannot move. It also means the recorded calls are genuine QNetworkRequest objects, so the tests assert on real URLs, headers and bodies - the contract spec/002_api.md specifies - rather than on arguments to our own wrapper. Delivery is explicit rather than automatic. Http connects to reply.finished *after* the manager hands the reply back, so a fake emitting on creation would be talking to nobody. Holding replies until deliver() also buys two things worth having: a test can inspect the state between a request going out and the answer arriving, and it can prove an action issued no request at all, which is the only way to state what the local search filter must not do. Routes come from the fixtures themselves. Each capture records the path that produced it, so the router normalises both sides - blanking id-shaped segments - and matches on the suffix. Nothing in the test code names a live id, and a re-capture against a different account re-routes itself. An unrouted request answers 501 and is recorded rather than raising, and a test asserts the unmatched list is empty. Without that the suite would pass on a plugin that quietly received errors for everything. Two corrections while writing the journey. The login button, not the dialog's accepted signal, is what triggers the exchange - the first version emitted accepted and no request was made at all. And the model combo is populated from a project's workflowDefs, not from the account response, so asserting it here was simply wrong about the plugin; that assertion moves to the projects journey. Also scrubs file checksums out of the fixtures. Not credentials, but detect-secrets reads a long hex string as one, so a captured checksum fails lint on every future capture. Blanked to the same length, which is idempotent and so joins --retrim. ## Manual test New behaviour: none - test-only. The plugin is untouched. Regression surface: none in the plugin. For the suite itself, the risk is a test that passes without proving anything, so if you want to check it bites: - break the authorization header in http.py and confirm test_login_sends_the_token_and_the_plugin_version fails - delete a fixture from responses/ and confirm test_every_request_during_login_has_a_captured_fixture names the missing path
…urally
The behavioral startup journey found a real defect on its first run, which is the argument
for the whole suite in one example.
ProvidersList.__getitem__ hands back NoneProvider for a negative index - a deliberate null
object for "nothing selected". But NoneProvider overrode only __bool__, so is_default,
requires_image_id, meta_url and preview_url all inherited the NotImplementedError versions
from ProviderInterface. A null object that raises on the questions its callers ask is not a
null object; it is a delayed crash.
on_provider_change asks exactly that question. On a fresh profile the provider combo is
empty, providerIndex() returns -1, and startup configuration died on
provider.requires_image_id. The error guard absorbed it, so the plugin came up looking
healthy with no model list, no imagery sources and no project table - the "on the surface it
continues working" failure, reached without any injected fault.
to_processing_params is left raising on purpose. Nothing can be processed without a real
source, and defaulting there would submit a job against imagery the user did not choose.
Why the combo was empty in the first place is a separate question and is now a WAL item: the
imagery sources are filtered by modelCombo.currentText(), but the model list arrives with a
project's workflowDefs rather than with the account response, so the 500 ms startup poll can
win the race against projects/{id}. On a fresh profile that may simply be the intended flow -
the user picks a project and both combos fill - so it is recorded as something to check on a
slow network with a saved project, not asserted as a defect.
The harness gained a settle() helper because most of the startup configuration hangs off that
timer: replies alone are not enough, real time has to pass. It waits rather than poking the
timer object, so it names nothing Phase C moves and will keep working when the poll belongs
to a session service.
Two assertions I got wrong and corrected rather than accommodated. The model and provider
combos are not populated by login at all - they come from a project - so asserting them in the
startup journey was a claim about the plugin that was simply false; they move to the projects
journey. And /rasters/memory is legitimately fetched twice during startup, once with the
account status and once when the mosaic list updates, so the test now asserts the count stops
growing rather than pinning a number that a good change would break.
## Manual test
New behaviour - the null-object fix is reachable whenever no imagery source is selected:
- start the plugin with an account that has a saved project, log in, and confirm the model
and provider combos populate and the processings table appears
- log in on a profile with no saved project (clear mapflow/project_id in QGIS advanced
settings) and confirm the projects table appears rather than an empty window; before this
it silently showed nothing
- open a project from that table and confirm models and imagery sources then populate
Regression surface: NoneProvider is handed out for any negative provider index, so anything
that reads the combo before a selection exists. Worth clicking through: switching data source
between Mapflow provider, My Imagery and imagery search; opening the plugin with no providers
configured; and starting a processing with no source selected, which must still be refused
rather than proceeding.
…ssing The plugin's spine, and the journey that unblocks the rest: nothing is reachable until a project is open, and opening one is what populates the model list and the imagery sources. Includes the failed-processing path, routed to the captured page that actually contains a FAILED row rather than relying on whichever project the capture happened to record first. Two things the journey exposed, neither of them a test artifact. Services are process-global singletons. ProviderService and AlertService cache their instance on the class, so get_instance returns the first one ever built and ignores the arguments of every later call - including the dialog. The instance keeps writing the imagery-source list into the dialog it was born with. The suite builds a plugin per test, so every journey after the first found an empty provider combo, and the same thing happens in QGIS after a plugin reload or an in-place upgrade: the user gets a plugin that cannot start a processing until QGIS is restarted. The conftest resets both classes to compensate, and a WAL entry carries the real fix into Phase C, where the plugin will own its services anyway. Opening a project depends on selection ordering. show_processings returns early unless a project id has been recorded, and that recording happens on the table's selection change, not in the double-click handler. A real double-click selects before it opens, so the plugin is correct - but a test that only emits doubleClicked is not driving what a user does. The helper now selects and then activates, which is also the honest description of the gesture. The harness gained fixture() for routing a journey to a specific captured variant, and the logged-in fixture now goes through initGui() and main() rather than reaching for the login dialog directly, so the journeys start where the user starts - at the toolbar button. Worth recording for whoever debugs the next one of these: the provider combo being empty was diagnosed by probing set_raster_sources directly, which worked fine, and that is what ruled out the dialog and pointed at shared state. Guessing from the symptom would have blamed the combo or the fixture data. ## Manual test Regression surface - nothing under mapflow/ changed in this commit, so the risk is only that the suite describes behaviour wrongly. Worth confirming by hand that the described flow is real: - open the plugin, log in, and confirm the projects table lists your projects - double-click a project and confirm the processings table fills, the model combo populates with the project's models, and the imagery sources appear - find a project containing a failed processing and confirm the failure is visible in the table as a status, not as an empty or blank row New behaviour: none. Tests only.
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
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.
Phase B of the 3.7.0 refactoring. These tests exist to answer one question with evidence
rather than assertion: did the refactoring change behaviour? So they may only name surfaces
that outlive it - the plugin entry point, widget object names from the .ui, the HTTP
conversation, QgsProject layers, QgsSettings keys. A behavioral test that needs editing
during Phase C has failed at its job, because the edit is where a behaviour change hides.
Feasibility was the open question and it is answered: Mapflow(iface) constructs and
initGui() runs inside the test container. The single accommodation is that
iface.mainWindow() must return a real QMainWindow - it becomes the parent of the main
dialog and of the plugin QObject, and PyQt rejects a MagicMock where it expects a QWidget.
That is why test_plugin_boots exists at all: a plugin that fails to construct does not
degrade, it silently never appears in QGIS, and it is the precondition for every later
journey.
The fixtures are captured from the live API rather than written from the spec. Hand-written
payloads contain only the fields we already knew about, so they cannot catch a parser
breaking on a field the server actually sends - which is exactly the regression class this
net is for. capture_fixtures.py discovers every id from an earlier response (list projects,
take an id, fetch that project) so it works against any account and hardcodes nothing.
Everything it sends is read-only except one opt-in call behind --include-create-failure,
which is shaped to be refused and prompts before running. Creating a processing spends real
credits, so that could not be the default.
The scrubber is tested rather than trusted, because it is the only thing between a real
account and a committed fixture. Consistency is the property that is easy to miss: a
project id must scrub to the same value in the list response and the detail response, or
the two fixtures cannot be used together in one test. Over-scrubbing is a failure too - a
fixture that has lost its statuses and model names is no longer a realistic payload - so
that is asserted as well.
detect-secrets flagged the invented credentials in those tests, correctly by its own rules.
The fix is a pragma on the literals plus a constant for the expected output, since
out["password"] == "redacted"reads as a hardcoded credential to the scanner and there isno reason to teach it otherwise.
Placed under tests/qgis/ rather than in a new tests/behavioral/ tier on purpose. A fourth
tier means editing the Makefile, and the Makefile is a watched file: any local edit blocks
every agent-make invocation until it reaches master, not just dev. That would stall the
whole refactor behind a human merge to the release branch. A subdirectory is collected by
the existing invocation, and it is the honest classification anyway - these need the QGIS
runtime, which is what the tiers actually split on.
Manual test
New behaviour - the capture tool, which is run by hand:
python3 tests/qgis/behavioral/capture_fixtures.py
request that produced it; the run prints what it saved, what it skipped and why, and a
count of what was scrubbed
payload shape it has not seen is passed through unchanged
stated reason rather than failing
Regression surface: none. Nothing under mapflow/ is touched; this adds test files only.