Skip to content

Commit 720c2cd

Browse files
committed
fix(provider-tck): read the canonical count from the assets, not from a test
The base branch followed the conformance assets to open-feature/spec@15fe861, which takes the canonical set from twenty-nine scenarios to forty. Three expectations here restated the old number and one declaration claimed a capability the provider behind InProcessControl does not have. - test_canonical_set derived nothing: two message assertions and one length assertion each named 29. The count now comes from canonical_scenarios() -- the same parse the guard itself runs -- so adding a canonical scenario does not require editing a test that exists to notice the addition. Appendix F makes that the rule, and a suite whose own tests have to be edited to follow the assets is a suite that will be edited to agree with them. - test_the_canonical_set_is_read_from_the_packaged_assets would have become vacuous if it compared canonical_scenarios() to itself, so it is measured against what pytest-bdd actually collected in the unfiltered run, less the three scenarios the generated extension contributes. That pins the parse to the runner rather than to a literal. - The generated adoptions in test_report declared NUMERIC_COERCION. The two lossless scenarios the new assets add are exactly the ones that catch a provider which does not coerce, and the provider behind InProcessControl does not: they failed. LARGE_INTEGERS is declared instead -- honest for a Python int, and still a third declared capability, which is all those tests needed. The declaration is corrected rather than the assertion weakened; finding 3 in the README is the same call for the in-memory self-tests. The README's figures follow: the self-test total, the example report line, the per-step status counts and the partial-run transcript are all re-measured. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
1 parent 3fce71e commit 720c2cd

3 files changed

Lines changed: 59 additions & 24 deletions

File tree

‎tools/openfeature-provider-tck/README.md‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -363,20 +363,20 @@ at `<dir>/<name>.ndjson`, which is a [Cucumber Messages][messages] stream.
363363

364364
```console
365365
$ PROVIDER_TCK_REPORT_DIR=./reports pytest
366-
provider-tck [in-memory]: report written to reports/in-memory.json with results in in-memory.ndjson (1 failed, 23 passed, 5 skipped)
366+
provider-tck [in-memory]: report written to reports/in-memory.json with results in in-memory.ndjson (1 failed, 28 passed, 11 skipped)
367367

368368
$ jq -c .results reports/in-memory.json
369369
{"format":"cucumber-messages","location":"in-memory.ndjson","digest":"sha256:c7e12a…"}
370370

371371
$ jq -r 'select(.testStepFinished) | .testStepFinished.testStepResult.status' \
372372
reports/in-memory.ndjson | sort | uniq -c
373373
1 FAILED
374-
220 PASSED
375-
45 SKIPPED
374+
281 PASSED
375+
91 SKIPPED
376376
```
377377

378-
Statuses are per step, not per scenario. Of the 45 skipped, 42 belong to the five scenarios the
379-
capability gate stopped — their before-hooks included, which is where the reason is — and three are
378+
Statuses are per step, not per scenario. Of the 91 skipped, 88 belong to the eleven scenarios the
379+
capability gate stopped — every step after the before-hook that carries the reason — and three are
380380
the steps of the failing scenario that were never reached.
381381

382382
It is an environment variable rather than a `TckConfig` field so that emitting a report is a property
@@ -397,11 +397,11 @@ execute all of them writes no report:
397397

398398
```console
399399
$ PROVIDER_TCK_REPORT_DIR=./reports pytest -k "unknown_flag_key"
400-
provider-tck [in-memory]: 28 of 29 canonical scenarios did not run, so this run cannot support a
400+
provider-tck [in-memory]: 39 of 40 canonical scenarios did not run, so this run cannot support a
401401
conformance claim and no report is written for it. …
402402
- features/errors.feature: A float flag is not silently narrowed to an integer
403-
- features/errors.feature: Requesting the wrong type returns the code default [key=float-flag requested=Boolean default=false]
404-
… and 18 more
403+
- features/errors.feature: An integer requested as a float is widened without loss
404+
… and 37 more
405405
```
406406

407407
A scenario the capability gate skipped **has** run: it was asked, and the report accounts for it
@@ -495,7 +495,7 @@ the field.
495495
| `test_canonical_set` | the canonical-set guard | a run that executed less than the canonical set fails and publishes nothing |
496496

497497
```
498-
155 passed, 21 skipped, 2 xfailed
498+
177 passed, 21 skipped, 2 xfailed
499499
```
500500

501501
No Docker and no network. The conformance suites take under a second; `test_report`,

‎tools/openfeature-provider-tck/tests/test_canonical_set.py‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import dataclasses
2222
import json
2323
import os
24+
import re
2425
import subprocess
2526
import sys
2627
import typing
@@ -113,12 +114,26 @@ def pytest_collection_modifyitems(items):
113114
Then the resolved details value should be "hi"
114115
"""
115116

117+
_EXTENSION_SCENARIOS = 3
118+
"""How many scenarios ``_VENDOR_FEATURE`` contributes."""
119+
120+
_OUTCOMES = re.compile(r"(\d+) (passed|failed|skipped|xfailed|xpassed)\b")
121+
"""The per-outcome counts of a ``-q`` summary line, which together are what ran."""
122+
116123
# The canonical scenario the filtered runs below leave out. Named rather than
117124
# counted, so a change to the canonical assets cannot leave these passing while
118125
# they select nothing.
119126
EXCLUDED_SELECTOR = "unknown_flag_key"
120127
EXCLUDED_SCENARIO = "An unknown flag key returns the code default"
121128

129+
# How many canonical scenarios there are, read from the packaged assets through
130+
# the same parser the guard itself uses. Appendix F makes that a rule: an
131+
# expectation about the canonical set that restates a number goes stale the next
132+
# time the specification adds a scenario, and a conformance suite whose own tests
133+
# have to be edited to follow the assets is a suite that will be edited to agree
134+
# with them.
135+
CANONICAL_COUNT = len(canonical_scenarios())
136+
122137

123138
@dataclasses.dataclass(frozen=True)
124139
class Run:
@@ -212,13 +227,16 @@ def test_the_whole_canonical_set_still_writes_a_report(complete: Run) -> None:
212227
def test_a_filtered_run_fails_and_writes_no_report(tmp_path: Path) -> None:
213228
"""The Go hazard, at the point it would have produced the document.
214229
215-
A well-formed report covering one scenario of twenty-nine is worse than no
216-
report, because nothing in it says which twenty-eight were never asked.
230+
A well-formed report covering one scenario of the canonical set is worse than
231+
no report, because nothing in it says which of the rest were never asked.
217232
"""
218233
run = _run(tmp_path, "-k", EXCLUDED_SELECTOR)
219234

220235
assert run.result.returncode != 0, run.stdout
221-
assert "of 29 canonical scenarios did not run" in run.stdout
236+
assert (
237+
f"{CANONICAL_COUNT - 1} of {CANONICAL_COUNT} canonical scenarios did not run"
238+
in run.stdout
239+
)
222240
assert not run.envelopes, "a partial run must publish nothing"
223241
assert not list(run.reports.glob("*.ndjson"))
224242
# The message names scenarios rather than only counting them, and says how
@@ -248,26 +266,36 @@ def test_acknowledging_a_partial_run_does_not_make_it_publishable(
248266
def test_an_extension_cannot_close_a_gap(tmp_path: Path) -> None:
249267
"""Three extension scenarios do not make up for one canonical one.
250268
251-
The adoption below runs thirty-one scenarios where the canonical set has
252-
twenty-nine, and is still one short: an adopter's scenarios are theirs, and
253-
counting them towards the specification's set would let any gap be filled by
254-
adding a feature file.
269+
The adoption below runs more scenarios than the canonical set contains and is
270+
still one short: an adopter's scenarios are theirs, and counting them towards
271+
the specification's set would let any gap be filled by adding a feature file.
255272
"""
256273
run = _run(tmp_path, "-k", f"not {EXCLUDED_SELECTOR}", extension=True)
257274

258275
assert run.result.returncode != 0, run.stdout
259-
assert "1 of 29 canonical scenarios did not run" in run.stdout
276+
assert f"1 of {CANONICAL_COUNT} canonical scenarios did not run" in run.stdout
260277
assert f"features/errors.feature: {EXCLUDED_SCENARIO}" in run.stdout
261278
assert not run.envelopes
262279

263280

264281
# -- what the canonical set is -----------------------------------------------
265282

266283

267-
def test_the_canonical_set_is_read_from_the_packaged_assets() -> None:
268-
"""One entry per Scenario Outline row, which is what a runner generates."""
284+
def test_the_canonical_set_is_read_from_the_packaged_assets(complete: Run) -> None:
285+
"""One entry per Scenario Outline row, which is what a runner generates.
286+
287+
The number is not written down here. It is read off the unfiltered run --
288+
pytest-bdd's own collection of the same packaged feature files, less the
289+
three scenarios the adopter's extension contributes -- because an
290+
expectation that restated a count would pin this file rather than the
291+
assets, and go stale the next time the specification adds a scenario.
292+
"""
269293
scenarios = canonical_scenarios()
270-
assert len(scenarios) == 29
294+
295+
summary = [line for line in complete.stdout.splitlines() if line.strip()][-1]
296+
collected = sum(int(count) for count, _ in _OUTCOMES.findall(summary))
297+
298+
assert len(scenarios) == collected - _EXTENSION_SCENARIOS, summary
271299
assert all(is_canonical_uri(uri) for uri, _, _ in scenarios)
272300
# An outline contributes rows, not a single templated entry.
273301
assert any(row for _, _, row in scenarios)

‎tools/openfeature-provider-tck/tests/test_report.py‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,15 @@ def tck_config():
136136
scenarios(features_path())
137137
'''
138138

139-
CAPABILITIES = "{Capability.EVENTS, Capability.OBJECT, Capability.NUMERIC_COERCION}"
140-
"""What the main generated suite declares: enough to produce a skip and a pass."""
139+
CAPABILITIES = "{Capability.EVENTS, Capability.OBJECT, Capability.LARGE_INTEGERS}"
140+
"""What the main generated suite declares: enough to produce a skip and a pass.
141+
142+
``LARGE_INTEGERS`` rather than ``NUMERIC_COERCION`` for the same reason the
143+
in-memory self-tests declare the one and not the other -- a Python ``int`` is
144+
unbounded, and the provider behind ``InProcessControl`` hands each variant back
145+
untouched rather than coercing it. Three declared capabilities are what these
146+
tests need; which three has to stay an honest claim about the provider.
147+
"""
141148

142149
NOT_APPLICABLE = '{Capability.STALE: "this provider has no connection to lose"}'
143150
"""One capability the provider cannot have rather than merely does not declare."""
@@ -588,7 +595,7 @@ def narrow_run(tmp_path_factory: pytest.TempPathFactory) -> Run:
588595
tmp_path_factory,
589596
file_name="narrow.json",
590597
name="narrow",
591-
capabilities="{Capability.NUMERIC_COERCION}",
598+
capabilities="{Capability.LARGE_INTEGERS}",
592599
not_applicable="{}",
593600
deviations=False,
594601
)
@@ -873,7 +880,7 @@ def test_the_declaration_is_an_input_not_a_summary(run: Run) -> None:
873880
declaration = run.envelope["declaration"]
874881
assert declaration["declared"] == [
875882
Capability.EVENTS.tag,
876-
Capability.NUMERIC_COERCION.tag,
883+
Capability.LARGE_INTEGERS.tag,
877884
Capability.OBJECT.tag,
878885
]
879886
assert declaration["notApplicable"] == {

0 commit comments

Comments
 (0)