Skip to content

refactor(tests): split the S3 stub router into one function per method - #334

Merged
BryanFRD merged 1 commit into
mainfrom
refactor/s3-stub-complexity
Sep 4, 2026
Merged

BryanFRD merged 1 commit into
mainfrom
refactor/s3-stub-complexity

Conversation

@BryanFRD

@BryanFRD BryanFRD commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

SonarQube flags stub() in the S3 test double at cognitive complexity 18 against a limit of 15, and it is right: the router closure carried the write rules, the read, the delete and the listing, each if costing double for sitting two levels deep inside an async closure.

The arms move out to put, head and delete, so the closure is a dispatch and each rule reads as a rule at the top level. Nothing changes about what the stub does, and its comments travel with the code they explain: why a conforming store refuses a mismatched checksum, why If-None-Match is the whole of lock uniqueness, why a delete answers 204 either way.

45 S3 tests pass, clippy clean.

@BryanFRD
BryanFRD enabled auto-merge (squash) September 4, 2026 16:32

@ferrfleet ferrfleet Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behaviour-preserving as far as I can read it. Each arm moved out unchanged: the checksum refusal, the If-None-Match precondition and the insert in put; the 204-either-way and the take-once arriving insert in delete; the content-length HEAD. &bytes still coerces from Bytes to &[u8] for matches, and &arriving from the Arc to &Mutex<_>.

Two things I checked because the refactor could have broken them quietly and neither did: the counted increment stays in the _ arm, so the "moves on GET, stays still for LIST, HEAD, PUT and DELETE" contract behind bucket_counting_key_reads holds; and the lock order in delete is still objects-inside-arriving, with nothing in the file taking them the other way.

Nit: put takes key: String while head and delete take &str. It is the right call (the insert wants ownership and a uniform &str would buy a clone), just worth knowing it is deliberate rather than drift.

Nit: the two adjacent bool parameters on put are the one thing here a caller could transpose without the compiler noticing. One call site, and it is correct, so this only matters if a second ever appears.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

SonarQube — aucune nouvelle issue

1 issue(s) corrigée(s) sur les fichiers touchés.

Comparaison entre le projet bac à sable de cette PR et la branche par défaut : SonarQube Community n'analyse pas les PR, ce delta est calculé côté CI. Détail

@BryanFRD
BryanFRD merged commit 419fe49 into main Sep 4, 2026
26 checks passed
@BryanFRD
BryanFRD deleted the refactor/s3-stub-complexity branch September 4, 2026 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant