update tests for the MyData api fix#451
Conversation
github-actions
Bot
added
FY26 Sprint 24
There was a problem hiding this comment.
Pull request overview
Updates the client’s integration and functional test suites for the MyData API behavior change where “no results” responses are now treated as successful (empty) results instead of errors (Issue #450, aligned with Dataverse PR IQSS/dataverse#12256).
Changes:
- Adjusts MyData-related tests to expect empty
MyDataCollectionItemSubsetresults instead ofReadErrors when filters yield no results. - Improves integration test setup/teardown robustness by using a unique MyData user and guarding cleanup steps.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/integration/collections/CollectionsRepository.test.ts | Updates MyData integration test setup/cleanup and changes expectations from error-on-empty to empty-subset success. |
| test/functional/collections/GetMyDataCollectionItems.test.ts | Reworks functional tests to assert empty-subset success across “no results” scenarios and updates related assertions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const actualCollectionPreview = actual.items[0] as CollectionPreview | ||
| expect(actualCollectionPreview.alias).toBe(testCollectionAlias) | ||
|
|
There was a problem hiding this comment.
I don't think the copilot suggested change is necessary, since the test only returns one item - ordering is not important.
| } catch (error) { | ||
| readError = error as ReadError | ||
| } finally { | ||
| expect(readError).toBeInstanceOf(ReadError) | ||
| expect(readError?.message).toEqual( | ||
| 'There was an error when reading the resource. Reason was: Sorry, no results were found.' | ||
| ) | ||
| console.log(error) | ||
| throw new Error('Item subset should be retrieved') | ||
| } |
There was a problem hiding this comment.
It looks like this try/catch logic is leftover from when we had to assert the message in the Error, for empty result sets. So I think the try/catch and throw new Error can be removed.
ekraffmiller
moved this from Ready for Review ⏩
to In Review 🔎
in IQSS Dataverse Project
ekraffmiller
left a comment
ekraffmiller
left a comment
There was a problem hiding this comment.
Hi @ChengShi-1 , looks good! Just one small suggested change.
What this PR does / why we need it:
Which issue(s) this PR closes:
Related Dataverse PRs:
Special notes for your reviewer:
Suggestions on how to test this:
Is there a release notes or changelog update needed for this change?:
Additional documentation: