Eliminate VCR from siade test suite - #378
Open
Samuelfaure wants to merge 30 commits into
Open
Conversation
Replace the two vcr cassettes for the certifications_qualiopi_france_competences endpoint with a ProviderStubs::CarifOref module and JSON payload fixtures, following the established WebMock stub pattern used by DGFIP and Qualifelec. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the insee/token cassette with the existing stub_insee_authenticate WebMock stub. Add an expires_in field to the stub response, required by AbstractGetToken#save_token_to_cache to compute a positive cache TTL - without it the Redis cache write is a no-op (TTL 0), which broke the "stores the new token... in cache" example. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the three insee/metadonnees cassettes with a ProviderStubs::INSEE
WebMock stub per fixture (one_result, no_result, multiple_results) and JSON
payload fixtures extracted from the cassette bodies, following the pattern
established for CARIF-OREF and INSEE Authenticate.
The no_result.json payload is intentionally empty: the recorded 404 cassette
body was an empty string, not JSON, so the fixture preserves that instead of
fabricating a shape.
In validate_response_spec.rb's "with real http calls" contexts, the stub is
registered via an `around` hook instead of `before`. The global
`config.before(type: :validate_response)` hook (spec_helper.rb) force-evaluates
the `response` let, which for these two contexts triggers a real HTTP call.
Global `before(:each)` hooks always run ahead of context-level `before(:each)`
hooks, so a plain `before { stub_... }` would register the stub too late and
the real request would go out unstubbed. `around` wraps the whole example,
including that global before hook, so the stub is in place before it runs -
this is exactly how the previous VCR cassette (activated via an around hook
too) avoided the same race.
Why: standardize siade's test-stubbing on one mechanism instead of splitting
between VCR and WebMock. VCR cassettes were legacy - recordings nobody
iterated on once made. WebMock-based ProviderStubs are easier to read and
iterate on directly in the codebase, and collecting every provider's real
response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/)
gives one centralized, browsable reference instead of cassettes scattered
and duplicated per test.
Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the 13 insee/siret cassettes with a ProviderStubs::INSEE stub per data case (active_GE, active_GE_ss, active_AE, closed, closed_without_date, non_diffusable, non_diffusable_ceased, gendarmerie_limousin, non_existent, redirected) and JSON payload fixtures extracted from the cassette bodies. Interactor-level specs (make_request_spec, build_resource_spec) stub only the siret GET, bypassing INSEE::Authenticate as before. Organizer and request-level specs compose stub_insee_authenticate with the matching data stub, reusing the same fixtures the interactor specs use (the _with_token cassettes recorded identical siret response bodies). The redirected case chains two stubs: a 301 with a Location header pointing at the new siret, then a 200 on that new siret's URL, mirroring the two real HTTP calls INSEE::Etablissement::MakeRequest#handle_redirect issues. The GEO_API commune lookups and secondary siren-based searches recorded in some cassettes (active_GE, active_GE_ss, active_AE, closed, non_diffusable*) are dead recording artifacts: none of the interactors or organizers under test ever call GEO_API or perform a siren search, and all specs stay green without stubbing them. Also migrates spec/interactors/insee/unite_legale/build_resource_spec.rb's one context depending on the same insee/siret/active_GE cassette, since it would otherwise break once the cassette directory is deleted. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
The local let(:redirected_siret) in the redirect context of make_request_spec.rb shadowed the pre-existing global helper of the same name (spec/support/helpers/siret.rb), which means the source siret that triggers a redirect. The local let redefined it to mean the destination siret instead, the opposite of its established meaning elsewhere in the suite. Rename it to destination_siret and use the real redirected_siret helper for the source siret. Also has stub_insee_etablissement_redirected reuse the existing stub_insee_siret_request helper for its second (200) stub instead of duplicating its body, and adds a comment explaining the two-hop case. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the 6 insee/siege cassettes with a ProviderStubs::INSEE WebMock stub per data case (active_GE, non_existent, non_diffusable_ceased, non_diffusable, gendarmerie_limousin, redirected) and JSON payload fixtures extracted from the cassette bodies. Unlike the siret endpoint, this is a search query (q=etablissementSiege:true AND siren:<siren>), so the new stubs match on query params via hash_including rather than a path segment. Despite its name, the "redirected" case is not an actual HTTP redirect: the recorded cassette is a single GET returning a plain 404, so its stub is a simple 404 response like the other not-found cases, not a two-hop 301 chain. Every context here, including at the interactor (make_request_spec) level, genuinely calls INSEE::Authenticate for real token, so every context composes stub_insee_authenticate with the matching data stub, unlike the Établissement specs where interactor-level specs bypassed auth with a hardcoded token. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the 11 insee/siren cassettes with a ProviderStubs::INSEE stub per
data case (active_GE, active_AE, ceased, non_existent, non_diffusable,
non_diffusable_ceased, gendarmerie_limousin, redirected) and JSON payload
fixtures extracted from the cassette bodies, following the same path-based
pattern as INSEE::Etablissement::MakeRequest (GET .../siren/{siren}).
Interactor-level specs (make_request_spec, build_resource_spec) stub only
the siren GET, bypassing INSEE::Authenticate as before. Organizer and
request-level specs compose stub_insee_authenticate with the matching data
stub, reusing the same fixtures (the _with_token cassettes recorded
identical siren response bodies).
The redirected case chains two stubs: a 301 with a Location header pointing
at the new siren, then a 200 on that new siren's URL, mirroring the two
real HTTP calls INSEE::UniteLegale::MakeRequest#handle_redirect issues. The
secondary siret/GEO_API lookups recorded in some cassettes are dead
recording artifacts: no interactor or organizer under test ever calls them,
and all specs stay green without stubbing them.
The pre-existing let(:redirected_siren) in make_request_spec.rb, which
shadows the global redirected_siren helper with the opposite meaning, is
left untouched as it predates this migration.
This removes the last insee/ cassettes, so spec/fixtures/cassettes/insee/
is deleted entirely. Also excludes spec/support/provider_stubs/insee.rb
from Metrics/ModuleLength, matching the existing exclusions for other
large spec/support files.
Why: standardize siade's test-stubbing on one mechanism instead of splitting
between VCR and WebMock. VCR cassettes were legacy - recordings nobody
iterated on once made. WebMock-based ProviderStubs are easier to read and
iterate on directly in the codebase, and collecting every provider's real
response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/)
gives one centralized, browsable reference instead of cassettes scattered
and duplicated per test.
Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Every other spec/support/provider_stubs/*.rb file that exceeds the Metrics/ModuleLength limit (cnav.rb, dgfip.rb, france_connect.rb, gip_mds.rb) disables the cop inline around the module body instead of being listed in .rubocop.yml. Match that convention for insee.rb. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the six VCR cassettes for AttestationsCotisationsRetraite and ConformitesCotisationsRetraite (a symlinked cassette directory) with a ProviderStubs::PROBTP module and JSON payload fixtures extracted from the cassette bodies, following the pattern established for CARIF-OREF and INSEE. ConformitesCotisationsRetraite::ValidateResponse declares type: :validate_response, so its three real-http-call contexts register the stub via an around hook instead of before, for the same reason documented in the INSEE migration: the global config.before(type: :validate_response) hook force-evaluates `response` (and thus `body`, which fires the real HTTP call) ahead of any example-level before hook. AttestationsCotisationsRetraite::ValidateResponse has no such type metadata, so its equivalent contexts use a plain before. The with_non_eligible_siret cassette under attestation/ was recorded but never referenced by any spec, so no payload fixture was extracted for it. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Add stub_inpi_rne_authenticate to ProviderStubs::INPI::RNE and use it in place of the 'inpi/rne/authenticate' cassette wherever a spec only needed the auth leg mocked (the six organizer specs, two of the three request specs, and the auth interactor spec's first context). The actes_bilans request spec's valid_siren and not_found_siren cassettes covered real HTTP responses instead: extract the valid_siren body into a fresh payload fixture (it did not match the existing curated actes_bilans/valid.json used by the organizer spec, so that helper is left untouched) and stub the not_found_siren 404 inline with no body, matching the sibling INPI request specs' not-found pattern. The proxy controller spec's actes_download/valid and bilans_download/valid cassettes each covered both the auth POST and the download GET, so their context now stubs both legs. Delete the now-unused spec/fixtures/cassettes/inpi directory. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the three qualibat cassettes (basic_auth token exchange, and the valid/not_found certificat responses) with a ProviderStubs::QUALIBAT WebMock stub module and payload fixtures, following the pattern established for CARIF-OREF, INSEE and PROBTP. The valid certificat response is a real PDF extracted from the cassette rather than a generic dummy: QUALIBATCertificationsBatimentExtractor parses the response body for API v4+, so a non-Qualibat PDF makes the extraction fail and breaks the schema-validated request spec. In validate_response_spec.rb's two vcr-tagged contexts that trigger a real Authenticate + MakeRequest chain via the response let, the stub is registered via an around hook instead of before, for the same reason as prior migrations: the global config.before(type: :validate_response) hook forces the response let (and the real HTTP call) before any example-level before(:each) would run. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
…tay real Content-meaningful provider PDFs that actually get parsed live under spec/fixtures/pdfs/<provider>_<resource>/ (see DGFIP's attestations fiscales), separate from the content-irrelevant dummy PDFs under spec/fixtures/payloads/pdf/. Qualibat already has spec/fixtures/pdfs/qualibat_certifications_batiment/ for the extractor spec; the valid_siret.pdf fixture belongs there too, read the same way DGFIP reads its fixtures (Rails.root.join(...).read) rather than via read_payload_file. Also document on stub_qualibat_valid_siret why this fixture can't be swapped for the shared pdf/dummy.pdf the way CIBTP's attestation stub does: Qualibat's v4 organizer actually parses this PDF's content via QUALIBATCertificationsBatimentExtractor, so a non-Qualibat PDF silently breaks extraction and only surfaces as a downstream schema-validation failure. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the 4 vcr cassettes for the extraits RCS and mandataires sociaux endpoints with a ProviderStubs::Infogreffe module and XML payload fixtures extracted from the cassette bodies, following the established WebMock stub pattern. Also drops the module's dead infogreffe_payload helper, which was never called and pointed at xml.erb templates that never existed. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the two vcr cassettes for the conventions_collectives endpoint with a ProviderStubs::FabriqueNumeriqueMinisteresSociaux module and JSON payload fixtures extracted from the cassette bodies, following the WebMock stub pattern established for CARIF-OREF and INSEE. In validate_response_spec.rb's two real-http-call contexts, the stub is registered via an `around` hook instead of `before`, since the global `config.before(type: :validate_response)` hook force-evaluates the `response` let (and thus the real HTTP call) before any example-level `before(:each)` hook would run. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the two vcr cassettes for the certifications_ingenierie endpoint with a ProviderStubs::OPQIBI module and payload fixtures extracted from the cassette bodies, following the established WebMock stub pattern. The 404 response body is plain text (Content-Type: text/html), so it's kept as .html rather than .json, matching the RNM precedent for non-JSON error bodies. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the mi/associations VCR cassettes with a ProviderStubs::MI module and JSON payload fixtures, following the established WebMock stub pattern. The DJEPVA unite_legale and unite_legale_open_data specs share the same provider under the hood, so their 404 contexts reuse the rna_not_found stub. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the four vcr cassettes for the douanes EORI endpoint with a ProviderStubs::DGDDI module and JSON payload fixtures, following the established WebMock stub pattern used by RNM and CARIF-OREF. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
ValidateSiretOrEORI rejects a malformed eori before MakeRequest ever runs, so the stub, fixture, and before hook for that context were never exercised. Follow the existing convention used for other local-validation failures (e.g. dgfip/attestation_fiscale_spec.rb) of registering no stub at all in that case. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace the three vcr cassettes for the certificats_rge endpoint with a
ProviderStubs::ADEME module and JSON payload fixtures, following the
established WebMock stub pattern used by CARIF-OREF and DGFIP.
The nested "with optional limit" rswag response block also required
registering both the valid_siret and valid_siret_with_limit stubs at the
outer response context: rswag's run_test! wires a
`before { submit_request(example.metadata) }` hook at each response
context level, and since it resolves example.metadata to whichever
example is currently running, the outer hook also fires an extra request
with the nested example's own params before the nested context's own
stub gets a chance to register.
Why: standardize siade's test-stubbing on one mechanism instead of splitting
between VCR and WebMock. VCR cassettes were legacy - recordings nobody
iterated on once made. WebMock-based ProviderStubs are easier to read and
iterate on directly in the codebase, and collecting every provider's real
response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/)
gives one centralized, browsable reference instead of cassettes scattered
and duplicated per test.
Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Reuse the existing ProviderStubs::URSSAF WebMock stubs (already backing ACOSS::AttestationsSociales under the hood) for ACOSS's own interactor, organizer and request specs, and drop the corresponding vcr: tags from URSSAF's specs. Add mock_urssaf_attestation_sociale_not_found for the FUNC517 "siren inconnu" case, extracted from the deleted cassette's real 400 response body. The valid-attestation stubs return a real PDF fixture (base64-encoded, matching ACOSS's wire format) already used elsewhere for this same domain. The Authenticate spec's expected token now matches what mock_urssaf_authenticate actually returns instead of the cassette's anonymized JWT. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Add stub_men_scolarites_valid_v2/not_found_v2 for the v2 GET endpoint and swap the remaining vcr-tagged contexts for WebMock stubs. Compose stub_men_scolarites_auth alongside the data stub in every request-spec context, since the full MEN::Scolarites organizer chain authenticates before fetching. The plain GET v2 stub reuses valid_v2_with_bourse.json rather than valid_v2.json: the latter was deliberately trimmed (no info-bourse) for an unrelated BuildResource unit test, and swapping it in here would have produced a null est_boursier, violating the response schema's nullable: false in request specs exercising a boursier scope. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Replace VCR cassettes with WebMock stubs backed by the existing bilans_entreprise_valid_data.json/bilans_entreprise_no_data.json payload fixtures. Add mock_not_found_banque_de_france for the not-found (code-retour 204) case. validate_response_spec.rb's real-http-response context uses an around hook instead of before, since the global config.before(type: :validate_response) hook force-evaluates response ahead of example-level before hooks. The request spec's 200 context referenced a vcr cassette_name that pointed at a nonexistent file; it already had manual stubs covering it, so the dead vcr key is simply dropped. Also remove retrieve_dgfip_dictionaries, an unused helper left over in this file that called into DGFIP internals unrelated to Banque de France stubbing. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Add stub_cibtp_attestation_cotisations_conges_payes_chomage_intemperies_missing_payments and _not_found to ProviderStubs::CIBTP, faithfully reproducing the provider's real 422 and 404 responses recorded in the deleted cassettes. Drop the remaining vcr: tags from the request spec and delete the cassette fixtures. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
All specs now use WebMock-based ProviderStubs and fixtures instead of VCR cassettes (spec/fixtures/cassettes was already empty and is now removed). - Delete spec/vcr_helper.rb and spec/support/activate_strict_vcr_request_matching_for_v3.rb - Remove the dead :disable_vcr metadata tag from the 5 DGFIP organizer specs - Remove the require 'vcr_helper' and the ActivateStrictVcrRequestMatchingForV3 include/extend/around hook from spec/spec_helper.rb - Remove the vcr gem from Gemfile and regenerate Gemfile.lock - Update siade/CLAUDE.md to drop the VCR-specific debug command and the now-inaccurate "VCR is legacy" framing - Fix spec/controllers/mcp_controller_spec.rb, which was accidentally relying on VCR's WebMock.net_connect_allowed? override (it always returns true while VCR is turned on) to let a real, unstubbed INSEE auth request reach DNS resolution and fail gracefully as a SocketError. Without VCR, WebMock blocks the same request earlier with NetConnectNotAllowedError, which the app doesn't rescue. Stub the INSEE::UniteLegale organizer instead of relying on that side effect. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
- README.md: drop the DEBUG_VCR debugging tip (a no-op now that vcr_helper.rb is gone) and reword the personal-data test warning to reflect that WebMock is the only stubbing mechanism, not a choice against VCR. - docs/development/utilisation_et_edition_credentials.md: stop referencing "cassettes VCR" when describing why some credential keys need specific test values; they're values expected by WebMock stubs. - spec/spec_helper.rb: remove the dead `unless ENV['regenerate_cassettes']` guard around the BanqueDeFrance/PROBTP http_options stubs. That env var only ever meant something to vcr_helper.rb's default_cassette_options (record: :new_episodes vs :none), which no longer exists, so the guard always evaluated true; the stubs now run unconditionally. - .rubocop.yml: drop the RSpec/Output exclude for the now-deleted spec/vcr_helper.rb. Why: standardize siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy - recordings nobody iterated on once made. WebMock-based ProviderStubs are easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads under spec/fixtures/payloads/ (and spec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test. Claude-Session: https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na
Member
Bah en fait sans instruction je relis pas ça hein :') |
Member
|
Est-ce que tu as effectué une analyse qualitative de l'intérêt à chaque étape ? In-fine le but du jeu est de pouvoir facilement itérer, si on n'a pas cette flexibilité, sur la plupart des API c'est (potentiellement ?) plus simple de rejouer des cassettes pour mettre à jour. En fait ça manque principalement d'un "why" on fait ça. |
Contributor
Author
|
@skelz0r J'ai relu, franchement y'a rien de complexe t'es même pas obligé de review, les cassettes ont été remplacées par des payloads, y'a quelques stubs qui ont été ajoutés et voilà (Pas énormément de code si tu skip les payloads en vrai) le "Pourquoi" pour moi c'est surtout de standardiser la stack, les cassettes VCR c'était surtout du legacy donc des trucs sur lesquels on itère pas, après les avantages des stubs par rapport à VCR c'est aussi en effet des itérations plus faciles et une référence complète centralisée des payloads IMO ça vaut largement la peine de merger |
Member
|
donc on relit commit par commit ? |
Member
|
j'aurais voulu quand même avoir une trace du "why" dans le code/commit, à minima dans la PR |
Contributor
Author
|
@skelz0r Pour la relecture, tu peux juste tout faire d'un coup en skippant les fichiers pas intéressants (en vrai ça va vite, zéro complexité) Je vais amend les commits pour réflect le "why" du coup |
Member
|
https://github.com/datagouv/apistration/pull/378/changes#diff-9a79bf1c27935bc01f2603e72001de21983d94d14214929446a4a0dd12c72f91R1 genre ça c'est pas très maintenable c'est assez painful à relire d'une traite, je vais faire commit par commit pour voir la tête de l'itération ça sera plus cohérent, je suis assez convaincu que c'est plus cohérent de lire par FD. |
Samuelfaure
force-pushed
the
enhance/kill_vcr
branch
from
September 2, 2026 08:08
6c433fa to
90c4129
Compare
Contributor
Author
|
@skelz0r C'est + maintenable que la cassette et je peut juste faire un petit commit pour auto indent le XML et on a le meilleur des deux mondes |
Member
d'où le "why", si c'est pour que ça soit plus maintenable c'est pas dans la promesse ici. d'ailleurs t'as hard migré mais y'avait peut-être moyen de faire plus simple pour infogreffe en bref ça se lit commit/commit imo |
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
2 participants
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.
Summary
Removes VCR (HTTP cassette recording/playback) entirely from
siade's RSpec suite, migrating every remaining provider spec to the WebMock-basedProviderStubs+ fixture pattern that had already been adopted for some providers (DGFIP, Qualifelec, etc.).siade/CLAUDE.mdalready said VCR was legacy and shouldn't be used for new work — this finishes that migration and deletes the gem/infrastructure so there's nothing left to accidentally reach for.spec/fixtures/payloads//spec/fixtures/pdfs/fixtures rather than fabricating new test data.spec/vcr_helper.rb,spec/support/activate_strict_vcr_request_matching_for_v3.rb,spec/fixtures/cassettes/, and thevcrgem fromGemfile/Gemfile.lock.spec/controllers/mcp_controller_spec.rbhad accidentally relied on a VCR-specific monkeypatch that let an unstubbed request silently succeed via a DNS failure; now explicitly stubbed.README.md, a credentials doc, a deadspec_helper.rbguard) left over from the main cleanup commit.Why
Standardize
siade's test-stubbing on one mechanism instead of splitting between VCR and WebMock. VCR cassettes were legacy — recordings nobody iterated on once made. WebMock-basedProviderStubsare easier to read and iterate on directly in the codebase, and collecting every provider's real response payloads underspec/fixtures/payloads/(andspec/fixtures/pdfs/) gives one centralized, browsable reference instead of cassettes scattered and duplicated per test.Test plan
bundle exec rspec→ 5419 examples, 0 failuresbundle exec rubocop→ clean, no offensesvcr:/cassette_name/disable_vcrreferences remain anywhere insiade/specVCRreferences remain inapp/,config/,lib/, or top-level docsspec_helper.rbregistrations, orphaned fixtures, commit history coherence)https://claude.ai/code/session_01P5BVeni5a7XSRmwWTPv8na