Commit a585469c authored by kzangeli's avatar kzangeli
Browse files

doubts: mark RESOLVED status + add #83 (HttpCtrl query-string non-GET) + #84 (post-MR cluster)



Bring testsuite-doubts.md fully up-to-date with the recent MRs and the
remaining unclear cluster surfaced by the post-MR ETSI run.

Status added to existing entries:

  #2  PARTIALLY RESOLVED via !284 (trailing-/ slice)
  #22 PARTIALLY RESOLVED via !284 (trailing-/ slice; query-params
      slice still open, see #83)
  #23 RESOLVED via !285 (25 distop tests: Should Be Equal
      -> Should Be Equal As Integers)
  #40 Slice #1 RESOLVED via !284; slices #2-3 still open, see #83
  #79 documented (no test-side fix yet — attrs= -> pick=)
  #80 RESOLVED via !283 (deleteAll= sweep)
  #80b documented (no fix yet — Create Subscription And Entity
       keyword default-arg bug)
  #82 documented (broker side done; test side still pending per item)

New entries:

  #83 HttpCtrl matcher's non-GET branch ignores the request's query
      string, so canonical ?options=… / ?deleteAll=… / ?sysAttrs=…
      forwards never match. Affects D003_02_red, D006_02_inc,
      D013_01_inc, D013_02_{exc,inc}.

  #84 Post-MR remaining ETSI cluster:
      84a D013_01_exc / D014_02_exc — § 9.3.3 fixture (same pattern
          as #82b on new tests)
      84b D013_01_red / D005_01_exc — § 5.9.2 CSR-vs-local-entity
          conflict (201 != 409)
      84c D014_02_inc — HttpCtrl 'NoneType' .get_url
          (downstream of #83's query-string mismatch)
      84d D005_01_inc / D005_01_red / D003_02_red — context-handling
          (#70 pattern on new tests)
      84e D018_02_02 — not triaged yet

Co-Authored-By: default avatarClaude Opus 4.7 <noreply@anthropic.com>
parent aed085be
Loading
Loading
Loading
Loading
+211 −0
Original line number Diff line number Diff line
@@ -93,6 +93,11 @@ when no stub matches, returning a default 404 immediately would let
broker-side error handling exercise (and would be obvious in test
logs).

**Status:** PARTIALLY RESOLVED in MR !284 — the specific trailing-`/`
slice of `__is_satisfy` (stub registered with trailing `/` never
matching a request with trailing `/`) is fixed there. The broader
substring-matching / odd-control-flow concerns remain.

---

## 3. Mock server started AFTER CSR registered
@@ -659,6 +664,15 @@ Robotframework-httpctrl supports `Set Stub Reply` with regex match
in newer versions — adopting that across the DistOps suite would
flip ~30 tests without touching the broker.

**Status:** PARTIALLY RESOLVED. The trailing-`/` slice — stub URL
ending in `/` not matching a request also ending in `/` — is fixed
in MR !284 (`__is_satisfy` now `rstrip("/")`-s both sides).
Broker-side `?pick=type[,scope],<attrs>` is emitted per § 6.3.4 + § 6.3.6
(swBroker commit 700d07f); HttpCtrl's query-string matching needs
further work to interpret it (substring matching mishandles the
canonical order). The wider regex/prefix proposal still stands for
the URL-params slice.


## 23. DistOps — `Get Stub Count 1 (integer) != 1 (string)` (~12 tests)

@@ -674,6 +688,9 @@ strict and fails.
**Fix wanted:** use `Should Be Equal As Numbers` / `Should Be Equal
As Integers`, or `Should Be True ${stub_count} == 1`.

**Status:** RESOLVED — fixed in MR !285 (25 test files; `Should Be
Equal``Should Be Equal As Integers`).


## 24. DistOps — `No keyword with name 'Get Request Url Params' found`

@@ -1110,6 +1127,12 @@ request to `/a/b/attrs` (missing trailing slash) or
  (c) switch tests that need precise URL matching to `Wait For
      Request` + manual reply instead of the stub mechanism.

**Status:** Slice #1 (trailing-`/` mismatch) RESOLVED in MR !284 —
`__is_satisfy` now strips trailing `/` from both sides of every
comparison. Slices #2 (`?sysAttrs=true` etc.) and #3 (`?options=…`,
`?deleteAll=…`) remain — those need HttpCtrl URL-params handling or
test-side stub registration to accept the canonical broker form.



## 41. `020_14_01/02` — default temporal page size is not in the spec
@@ -2505,6 +2528,9 @@ The spec could be clearer that distributed-operation forwards must
translate any legacy `attrs=` from a 1.6-era CSR into `pick=` so that mock
test stubs written against the modern spec match the forwarded URL.

**Status:** documented (no fix yet) — the test still needs to flip
`attrs=speed``pick=speed` in `D011_02_exc.robot`.

## 80. `046_22_08` — `deleteAll=${true}` serializes as `deleteAll=True` (capital T)

**Files:**
@@ -2591,6 +2617,14 @@ delete cycle is consistent. But:
The second form matches the convention already used elsewhere in the
suite (`deleteAll=${EMPTY}` etc.).

**Status — 80:** RESOLVED — `deleteAll=${true}` part fixed in MR !283
(same MR that swept all `local=${True}` URL params); the broker now
gets a lowercase `true` and the test progresses past the parseBool 400.

**Status — 80b:** documented (no fix yet) — the `${entity_id_suffix}`
default-arg bug in `Create Subscription And Entity` is still on the
queue. Low priority since the tests "work" by accident.

## 82. `D010_01_exc` / `D002_02_exc` / `D003_02_exc` / `D004_01_exc` — distop tests surfaced as test-side after § 9.3.3 enforcement

The triage of four distop "later error" failures (from the `lowercase
@@ -2667,3 +2701,180 @@ doesn't match the URL id.
`$.id` with the actual `${entity_id}` before PATCHing. Or omit the
`id` from the fragment entirely (PATCH attrs doesn't need it).

**Status — 82a/82b/82c/82d:** documented (no test-side fix yet).
The broker-side enforcement that surfaced each item is upstream:
swBroker commits a0384f3 (§ 9.3.3 exclusive structural validation)
+ 700d07f (forward `pick=type[,scope]` with id-reassemble) +
ldHooks Link-header @context propagation. Each item still needs its
own test-suite-side fix per the recipe in the entry above.

## 83. HttpCtrl matcher accidentally couples `?options=…` / `?sysAttrs=true` etc. to GET-only sub-branches

**Library:** `libraries/robotframework-httpctrl/src/HttpCtrl/http_stub.py`,
`HttpStubContainer.__is_satisfy`.

**Hit:** Even after the trailing-`/` fix (MR !284), a forward whose
URL carries the canonical NGSI-LD URL params still misses its stub
on **non-GET** methods. The matcher has two branches inside
`if '?' in criteria.url:`:

- GET branch (lines ~84–126): walks both the stub and request
  query strings, validates the stub's parameters appear in the
  request, and accepts the match.
- Non-GET branch (lines ~128–144): only verifies
  `stub.criteria.url == criteria_url_components[0]` — the stub URL
  WITHOUT a `?` against the request URL minus its `?…` tail. If the
  stub registers any query at all, the non-GET branch won't take it.

This affects all write-op forwards that the broker (correctly per
§ 10.2.4 / § 10.2.7 / § 10.3.4) emits with a query string:
`?options=noOverwrite`, `?options=update`, `?deleteAll=true`,
`?sysAttrs=true`, …

**Concrete hits in the swBroker ETSI run:**

- `D003_02_red``POST /attrs/?options=noOverwrite`
- `D006_02_inc``DELETE /attrs/speed?deleteAll=true`
- `D013_01_inc``POST /entityOperations/upsert?options=replace`
- `D013_02_exc` / `D013_02_inc``?options=update`

In each case the broker forwards a spec-correct URL and HttpCtrl's
non-GET branch falls through without matching. Mock either times
out or returns its default, broker reports `207` instead of `204`
(or `502` instead of `200`).

**Spec:** TS 104-175 § 10.2.4.4, § 10.2.7.4, § 10.3.4.4 require these
params to be propagated end-to-end on distop forwards.

**Our call:** broker uninvolved — emits canonical URL.

**Fix wanted:** mirror the GET-branch parameter-walk in the non-GET
branch, OR adopt the wider regex/glob proposal already noted in #22
/ #40 (slice 2-3).

## 84. New ETSI failures cluster — invalid-fixture / context / HttpCtrl AttributeError

**Background.** After MRs !283 (lowercase URL bool params), !284
(HttpCtrl trailing-`/` matcher), !285 (`Should Be Equal`
`As Integers`) all landed and the swBroker side already enforces
§ 9.3.3 + forward-pick `type[,scope]` (commits a0384f3 + 700d07f),
the remaining "Unclear / needs triage" cluster from the swBroker
ETSI run reduced to six tests with three distinct test-side root
causes (broker is correct in all six):

**84a) `D013_01_exc` / `D014_02_exc` — fixture invalid for exclusive (§ 9.3.3 surface, new tests)**

Same pattern as #82b but on different tests. Setup builds an
exclusive CSR from `csourceRegistrations/context-source-registration-vehicle-redirection-ops.jsonld`
— a type-only `{"entities":[{"type":"Vehicle"}]}` selector with no
attribute names. Per § 9.3.3 an exclusive registration shall define
BOTH a specific entity id AND propertyNames / relationshipNames; a
spec-strict broker rejects it with `400 BadRequestData`. Test
expects `201` and the setup `Check Response Status Code 201` fails
(`201 != 400`).

**Fix:** give the test its own fixture with id + propertyNames
matching what the test exercises (same recipe as #82b).

**84b) `D013_01_red` / `D005_01_exc` — CSR creation conflict with local entity (`201 != 409`)**

Setup creates the entity locally with `local=true`, then registers
a redirect / exclusive CSR for the same entity id. Per § 5.9.2 the
broker checks for overlap between the new exclusive/redirect CSR
and any locally-held data: if the local entity already carries the
attrs the CSR claims, the broker returns `409 AlreadyExists`. Test
expects `201`.

The original test design relied on the pre-§ 9.3.3 broker
behaviour where CSRs that only specified `type: Vehicle` and no
attribute set didn't trigger the conflict check (nothing to
compare). After § 9.3.3 the CSR must carry propertyNames, and as
soon as those propertyNames overlap the local entity's attrs the
conflict check fires.

**Fix:** seed the local entity with attrs that do NOT overlap the
CSR's claim, OR create the CSR before the entity, OR drop the
`local=true` on entity creation so the broker's distop dispatcher
takes ownership.

**84c) `D014_02_inc` — HttpCtrl `'NoneType' object has no attribute 'get_url'`**

When the broker's forward to the CSR's mock takes a path HttpCtrl's
`__is_satisfy` doesn't match, the handler falls through to
`ResponseStorage().pop()` — but for this test the storage is empty
and pop returns `None`. The next `response.get_url()` raises
`AttributeError`. This is the "blocking fallback" side-issue
already called out in #2 surfacing as a None-deref rather than a
hang. Likely cause: same query-string mismatch as #83 (`?options=update`
on update-batch forwards). Once #83 is fixed this should clear.

**Fix wanted:** test-side stubs aligned with what the broker emits
once #83 lands.

**84d) `D005_01_inc` / `D005_01_red` / `D003_02_red` — expanded IRI in response (#70 pattern on different tests)**

Body assertion `Dictionary does not contain key 'speed'` because
the response returns `https://ngsi-ld-test-suite/context#speed`
(the test-suite-context expansion). Same root cause as #70: the
test sends `Content-Type: application/json` to the broker without
a Link header, so the broker uses core context; CSR was registered
with test-suite-compound context; the two namespaces don't reconcile
in the response.

**Fix:** add the test-suite-context Link header on the test's main
call (or move to `application/ld+json` with a root `@context`).

**84e) `D018_02_02` — `204 != 207`**

Not triaged in detail yet — left for a follow-up run.

**84f) `D016_01_red` — test forgets to stub the GET for the post-merge query**

The test merges two entities via a redirect CSR (stubs for
`POST /broker{1,2}/.../entityOperations/merge` are registered). Both
merge forwards return 204 successfully. The verification step then
calls `Query Entities entity_types=Vehicle ...` — but the test never
stubs `GET /broker{1,2}/.../entities?type=Vehicle&id=…`. With redirect
mode the broker holds no local data, so it forwards the query to both
CSes; neither stub matches; both forwards time out; the broker merges
zero results and returns `[]`. The assertion
`Should Contain ${response.json()} speed` fails with
`'[]' does not contain 'speed'`.

**Fix:** add stubs for the `GET /broker{1,2}/ngsi-ld/v1/entities?…`
that return a merged-entity payload containing `speed`. Mirror what
`D016_01_inc` does (which stubs both legs).

**84g) `D001_01_inc` — joins #72 (leftover Vehicle entities in current state)**

Full-suite failure mode `Lengths are different: 1 != 5`. The
`Query Entities entity_types=Vehicle local=true` returns 5 entities —
one created by this test plus 4 leftovers from earlier
DistOps tests in the full run (mirrors the cluster already documented
in #72: `D001_02_inc / D001_03_03_inc / D011_02_inc`). Passes in
isolation. Same fix as #72.

**84h) `D007_01_exc` — Robot keyword choice doesn't tolerate the spec-mandated absence of speed**

After PUT-Replace + an exclusive CSR claiming `speed`, the local
entity correctly drops the claimed attr per § 4.3.6.3 — the local
copy carries `{id, type, isParked2}` and no `speed`. The test then
does

```robot
${body}=    Get From Dictionary    ${response.json()}    speed
Should Not Contain    ${body}    speed
```

`Get From Dictionary` raises when the key is absent, so the test
fails on line 40 with `Dictionary does not contain key 'speed'`
even though that's exactly the broker behaviour the test is meant
to verify.

**Fix:** use `Dictionary Should Not Contain Key    ${response.json()}
speed` instead of fetching the value and substring-checking it. The
extracted-then-substring-checked pattern only makes sense if you
expect the key to be present and want to verify it equals
something — not the negative-presence assertion this test is
trying to do.