Skip to content

test: hardening CI path semantics per candidate depth-2, sources depth-3, support dataset#61

Merged
Gabrymi93 merged 2 commits intomainfrom
test/hardening-ci-path-semantics
Mar 21, 2026
Merged

test: hardening CI path semantics per candidate depth-2, sources depth-3, support dataset#61
Gabrymi93 merged 2 commits intomainfrom
test/hardening-ci-path-semantics

Conversation

@matteocavo
Copy link
Copy Markdown
Contributor

Closes #54

Cosa cambia

toolkit/core/config_models.py

  • aggiunge repo_root come parametro opzionale in DatasetConfig
  • se passato, valida che effective_root resti dentro la repo e fallisce con errore esplicito se esce
  • docstring esplicito: il guard è pensato per caller esterni (CI di monorepo come dataset-incubator), non è un cambio di semantica globale del toolkit

toolkit/core/config.py

  • espone repo_root tramite load_config(..., repo_root=...)

tests/test_config.py

  • layout reali coperti: candidate depth-2, sources/* depth-3, support dataset
  • path assoluto dentro repo: ammesso
  • path fuori repo: fallisce con errore esplicito
  • comportamento senza guard: invariato (retrocompatibilità)

Perché

Il bug post-rename in dataset-incubator (PR #52) ha dimostrato che la semantica di effective_root può rompersi silenziosamente. Il guard aggiunge enforcement opzionale nei contesti CI senza toccare il comportamento standard.

Il collegamento operativo lato dataset-incubator è tracciato in dataciviclab/dataset-incubator#82 (già implementato in PR #83).

Verifica

  • pytest: 184 passed
  • ruff: pulito

…h-3, support dataset

- aggiunge guard opzionale repo_root in DatasetConfig per validare
  che effective_root resti dentro la repo in contesti CI
- espone il guard tramite load_config(..., repo_root=...)
- aggiunge docstring esplicito: il guard è pensato per caller esterni
  (es. CI di monorepo come dataset-incubator), non per uso generale
- copre con test i layout reali: candidate depth-2, sources/* depth-3,
  support dataset, path assoluto dentro repo, errore fuori repo,
  retrocompatibilità senza guard

Closes #54
@matteocavo matteocavo added the enhancement New feature or request label Mar 21, 2026
@matteocavo matteocavo self-assigned this Mar 21, 2026
@matteocavo matteocavo added the enhancement New feature or request label Mar 21, 2026
@matteocavo matteocavo requested a review from Gabrymi93 March 21, 2026 08:07
@matteocavo
Copy link
Copy Markdown
Contributor Author

@Gabrymi93 pronta per review.

Il guard repo_root è opzionale — zero behavior change per i caller esistenti. Copre i layout reali dell'incubator (candidate depth-2, sources depth-3, support dataset).

Note pre-review:

  • il codice reviewer interno ha segnalato 2 HIGH da valutare: doppia resolve in _ensure_root_within_repo e assenza di check is_dir() su repo_root. Possiamo fixarli prima o durante la review, come preferisci.

Closes #54

- _ensure_root_within_repo ora riceve path già risolti dal caller
  (docstring esplicito, no double expanduser/resolve nel guard)
- call site in load_config_model risolve repo_root prima di passarlo
- aggiunto check is_dir() su repo_root con errore esplicito se il
  path non esiste o non è una directory
@matteocavo
Copy link
Copy Markdown
Contributor Author

Review mirata su `repo_root` guard e test path semantics fatta.

Esito:

  • nessun finding HIGH/CRITICAL su questo scope
  • ok la rimozione della doppia `resolve()` dentro `_ensure_root_within_repo`
  • ok il check esplicito `repo_root.is_dir()` alla call site
  • copertura test sufficiente per:
    • candidate depth-2
    • sources depth-3
    • support_datasets
    • root assoluto valido dentro repo
    • root fuori repo con guard
    • retrocompatibilità senza guard

Verifiche locali:

  • `pytest tests/test_config.py -q` → 57 passed
  • `ruff check toolkit/core/config.py toolkit/core/config_models.py tests/test_config.py` → pulito

Nota non bloccante:

  • manca un test dedicato al ramo `repo_root does not exist or is not a directory`, eventualmente da aggiungere dopo il merge

Copy link
Copy Markdown
Member

@Gabrymi93 Gabrymi93 left a comment

Choose a reason for hiding this comment

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

Top!

@Gabrymi93 Gabrymi93 merged commit 4f443e9 into main Mar 21, 2026
5 checks passed
@Gabrymi93 Gabrymi93 deleted the test/hardening-ci-path-semantics branch March 21, 2026 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: hardening CI path semantics per candidate depth-2, sources depth-3, support dataset

2 participants