Skip to content

Trashmodalunit test#5816

Open
Prashant-thakur77 wants to merge 8 commits intolearningequality:unstablefrom
Prashant-thakur77:trashmodalunit-test
Open

Trashmodalunit test#5816
Prashant-thakur77 wants to merge 8 commits intolearningequality:unstablefrom
Prashant-thakur77:trashmodalunit-test

Conversation

@Prashant-thakur77
Copy link
Copy Markdown
Contributor

@Prashant-thakur77 Prashant-thakur77 commented Apr 7, 2026

Summary

Migrates trashModal.spec.js from @vue/test-utils to @testing-library/vue, as part of the broader effort to make Studio's frontend tests more user-centric and resilient to implementation changes.
Manual verification:

  • Logged in as a@a.com, opened a published channel, removed resources, and verified the trash modal behavior .
  • All the test cases passed.
Screencast.From.2026-04-07.16-36-39.webm

References

Closes #5790
See #5789

Reviewer guidance

1)Only trashModal.spec.js was changed.
2) Reviewer need to check if all the needed testcases are present and testing-library is properly used.

AI usage

I used Claude to help plan and implement the migration.I checked other Testing-library implementations and verified the tests written and updated wherever needed.I followed the guidance given in the issue description and its parent issue description.I again checked if all the test cases are passing properly.

@learning-equality-bot
Copy link
Copy Markdown

👋 Hi @Prashant-thakur77, thanks for contributing!

For the review process to begin, please verify that the following is satisfied:

  • Contribution is aligned with our contributing guidelines

  • Pull request description has correctly filled AI usage section & follows our AI guidance:

    AI guidance

    State explicitly whether you didn't use or used AI & how.

    If you used it, ensure that the PR is aligned with Using AI as well as our DEEP framework. DEEP asks you:

    • Disclose — Be open about when you've used AI for support.
    • Engage critically — Question what is generated. Review code for correctness and unnecessary complexity.
    • Edit — Review and refine AI output. Remove unnecessary code and verify it still works after your edits.
    • Process sharing — Explain how you used the AI so others can learn.

    Examples of good disclosures:

    "I used Claude Code to implement the component, prompting it to follow the pattern in ComponentX. I reviewed the generated code, removed unnecessary error handling, and verified the tests pass."

    "I brainstormed the approach with Gemini, then had it write failing tests for the feature. After reviewing the tests, I used Claude Code to generate the implementation. I refactored the output to reduce verbosity and ran the full test suite."

Also check that issue requirements are satisfied & you ran pre-commit locally.

Pull requests that don't follow the guidelines will be closed.

Reviewer assignment can take up to 2 weeks.

@Prashant-thakur77
Copy link
Copy Markdown
Contributor Author

Hlo @MisRob ,I have resolved the issue,do review when you have time.

@learning-equality-bot
Copy link
Copy Markdown

📢✨ Before we assign a reviewer, we'll turn on @rtibblesbot to pre-review. Its comments are generated by an LLM, and should be evaluated accordingly.

@learning-equality-bot
Copy link
Copy Markdown

📢✨ Before we assign a reviewer, we'll invite community pre-review. See the community review guidance for both authors and reviewers.

Copy link
Copy Markdown
Member

@AllanOXDi AllanOXDi left a comment

Choose a reason for hiding this comment

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

Hi @Prashant-thakur77, thanks for working on this. I left a few clean up

.mockResolvedValue({});
jest.spyOn(TrashModal.methods, 'loadAncestors').mockResolvedValue();
jest.spyOn(TrashModal.methods, 'removeContentNodes').mockResolvedValue();
const loadNodesSpy = jest.spyOn(TrashModal.methods, 'loadNodes');
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.

This works but is non-obvious; add a comment explaining that real execution is intentional.

jest.spyOn(TrashModal.methods, 'loadChildren').mockReturnValue(new Promise(() => {}));
} else {
jest.spyOn(TrashModal.methods, 'loadChildren').mockResolvedValue({
more: items === testChildren ? null : { parent: TRASH_ID, page: 2 },
Copy link
Copy Markdown
Member

@AllanOXDi AllanOXDi Apr 15, 2026

Choose a reason for hiding this comment

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

using (===) to decide pagination is not quite right "show more unless the exact same array object was passed." Use an explicit hasMore parameter instead.

expect(wrapper.vm.selected).toEqual([]);
expect(wrapper.vm.previewNodeId).toBe(null);
await waitFor(() => {
expect(loadContentNodesSpy).toHaveBeenCalledWith({ parent: TRASH_ID, page: 2 });
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.

The assertion uses toHaveBeenCalledWith, which passes if any call matches, so the initial call and the "Show more" call are both included. This passes correctly but could be more precise with toHaveBeenLastCalledWith to explicitly assert the pagination call was the most recent one.


await user.click(showMoreBtn);

it('moveNoves should clear selected and previewNodeId', async () => {
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.

The original had a test for clicking an item, setting previewNodeId (which opens ResourceDrawer). This has been dropped entirely. It should either be added back or explicitly noted in the PR description as an intentional omission.

import { RouteNames } from '../../../constants';
import TrashModal from '../TrashModal';

const store = factory();
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.

Move inside makeWrapper so each test gets a clean store.

const utils = render(
TrashModal,
{
store,
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.

Passing the shared module-level store into render. See line 9, the shared store's dispatch spy in the deletion test ( see line 200) may not be fully cleaned up before subsequent tests run.

expect(screen.getByTestId('delete')).toBeDisabled();
expect(screen.getByTestId('restore')).toBeDisabled();

await user.click(screen.getAllByRole('checkbox')[1]);
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.

A bit brittle. [1] assumes the second checkbox in DOM order is the first item checkbox (index 0 being select-all). If any component renders an additional checkbox before the list, this clicks the wrong element. Use screen.getAllByTestId('checkbox')[0] instead.


await waitFor(() => {
screen
.getAllByRole('checkbox')
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.

Relies on the select-all being first in DOM order. Works today but fragile for the same reason as line 117.


it('successful deletion triggers snackbar and reloads nodes', async () => {
jest.spyOn(TrashModal.methods, 'deleteContentNodes').mockResolvedValue();
const dispatchSpy = jest.spyOn(store, 'dispatch').mockImplementation(() => Promise.resolve());
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.

Since makeWrapper is called after this line, the spy is active during mount. But because the store is shared (line 9), this mock intercepts dispatch calls in other tests if cleanup races. The fix is again to move store into makeWrapper.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Convert trash modal unit tests to Vue Testing Library

4 participants