Skip to content

Build expected metadata with the server's DANDI schema version - #1894

Merged
yarikoptic merged 1 commit into
masterfrom
use-server-schema-version-in-expected-metadata
Jul 28, 2026
Merged

Build expected metadata with the server's DANDI schema version#1894
yarikoptic merged 1 commit into
masterfrom
use-server-schema-version-in-expected-metadata

Conversation

@candleindark

@candleindark candleindark commented Jul 27, 2026

Copy link
Copy Markdown
Member

Problem

test_update_dandiset_from_doi fills the schemaVersion and @context of its expected metadata from the DANDI_SCHEMA_VERSION of the client's dandischema, but in the served metadata under test both carry the server's. While dandi-cli and the dandi-archive instance are built against the same dandischema the two coincide and the test passes; once they differ, every parametrization fails on those two fields even when the DOI-derived metadata the test exists to check is correct. The test thus asserts something it was never meant to: that both sides run the same DANDI_SCHEMA_VERSION.

Fix

Build those two fields from the schema version reported by the server's /info/, as the test already does for the other instance-dependent fields (url, repository, manifestLocation).

Where this surfaced

dandi/dandi-schema#419 bumps DANDI_SCHEMA_VERSION to 0.8.0. In that repo's test-dandi-cli matrix, the schema_install: server job installs the branch into the API image only, leaving a released client against a 0.8.0 server. All five parametrizations then fail, while the other 1001 tests in the job pass:

{'schemaVersion': '0.8.0'} != {'schemaVersion': '0.7.0'}
{'@context': '.../releases/0.8.0/context.json'} != {'.../releases/0.7.0/context.json'}

Test plan

`test_update_dandiset_from_doi` builds the expected metadata with the
`DANDI_SCHEMA_VERSION` of the client's `dandischema`, while the value in the
metadata under test is the server's. The test therefore incidentally requires
dandi-cli and the dandi-archive instance to carry the same
`DANDI_SCHEMA_VERSION`. Retrieve it from the server's `/info/` instead.

Co-Authored-By: Claude Code 2.1.220 / Claude Opus 5 <noreply@anthropic.com>
@candleindark candleindark added the tests Add or improve existing tests label Jul 27, 2026
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.94%. Comparing base (0dce272) to head (293da2e).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1894      +/-   ##
==========================================
- Coverage   76.96%   76.94%   -0.02%     
==========================================
  Files          88       88              
  Lines       12882    12882              
==========================================
- Hits         9914     9912       -2     
- Misses       2968     2970       +2     
Flag Coverage Δ
unittests 76.94% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yarikoptic yarikoptic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hm, I think it does make more sense this way! Nice find! Most likely we had this test created BEFORE we exposed version of the schema used by the server. So let's proceed this way indeed.

@yarikoptic yarikoptic added the release Create a release when this pr is merged label Jul 28, 2026
@yarikoptic
yarikoptic merged commit 4b02181 into master Jul 28, 2026
40 of 42 checks passed
@yarikoptic
yarikoptic deleted the use-server-schema-version-in-expected-metadata branch July 28, 2026 01:20
@github-actions

Copy link
Copy Markdown

🚀 PR was released in 0.76.8 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release Create a release when this pr is merged released tests Add or improve existing tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants