Skip to content

Comments

Add tests for file deletion in files reducer#3853

Open
aashu2006 wants to merge 3 commits intoprocessing:developfrom
aashu2006:test-files-delete
Open

Add tests for file deletion in files reducer#3853
aashu2006 wants to merge 3 commits intoprocessing:developfrom
aashu2006:test-files-delete

Conversation

@aashu2006
Copy link
Contributor

@aashu2006 aashu2006 commented Feb 9, 2026

Changes:
Refactored and expanded reducer tests for the DELETE_FILE action in client/modules/IDE/reducers/files.js.

  • Added comprehensive test coverage for file and folder deletion.
  • Covered nested folder hierarchies and empty folders.
  • Verified that sibling files remain unaffected.
  • Consolidated redundant tests and removed CREATE_FILE usage in setup.
  • Introduced a helper to construct reducer state directly for better isolation.
  • Added parentId to test state to mirror production shape.

Accessibility: Not applicable, this PR adds tests only and does not modify UI, markup, or styles.

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • has no typecheck errors (npm run typecheck)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number
  • meets the standards outlined in the accessibility guidelines

@aashu2006
Copy link
Contributor Author

Taking up @clairep94 suggestion to add more test coverage to the web editor. Beginning with the file deletion logic in the files reducer. 🙂

Copy link
Collaborator

@clairep94 clairep94 left a comment

Choose a reason for hiding this comment

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

Small nits, and potentially some tests you can delete.

I think re: not using CREATE_FILE in your setup, you could do something like:

// helper function above your tests
const createTestState = (files) => {
return [...initialState(), ...files] // or whatever equivalent with push, to your preference
}


// tests
it('blah blah blah I need two extra files in addition to the initialState to start' () => {
let state = createTestState([
  {
    id: 'firstFile',
    _id: 'firstFile,
    name: 'firstFile',
    content: '',
    children: [],
    fileType: 'file',
    parentId: rootId
  },
  {
    id: 'secondFile',
    _id: 'secondFile',
    name: 'secondFile',
    content: '',
    children: [],
    fileType: 'file',
    parentId: rootId
  },
])

// rest of your test
})

Great stuff though, thanks for doing this and it was very easy to read

@aashu2006
Copy link
Contributor Author

Small nits, and potentially some tests you can delete.

I think re: not using CREATE_FILE in your setup, you could do something like:

// helper function above your tests
const createTestState = (files) => {
return [...initialState(), ...files] // or whatever equivalent with push, to your preference
}


// tests
it('blah blah blah I need two extra files in addition to the initialState to start' () => {
let state = createTestState([
  {
    id: 'firstFile',
    _id: 'firstFile,
    name: 'firstFile',
    content: '',
    children: [],
    fileType: 'file',
    parentId: rootId
  },
  {
    id: 'secondFile',
    _id: 'secondFile',
    name: 'secondFile',
    content: '',
    children: [],
    fileType: 'file',
    parentId: rootId
  },
])

// rest of your test
})

Great stuff though, thanks for doing this and it was very easy to read

Thanks for the suggestion! I refactored the tests to avoid using CREATE_FILE in setup.

Instead of dispatching actions, I added a small createTestState helper to construct the reducer state directly, so the DELETE_FILE tests are now fully isolated from other reducer logic. I also consolidated a couple of redundant tests while making these changes. Let me know if you would prefer a different approach for the helper 🙂

@aashu2006
Copy link
Contributor Author

I have made the changes based on @clairep94 feedback:

  • Removed all CREATE_FILE usage in test setup
  • Built test state directly to keep DELETE_FILE isolated
  • Consolidated redundant tests (6 → 4)
  • Merged sibling and state-consistency checks into existing tests
  • Added parentId to better match production state shape.

Everything is passing locally. Please let me know if anything else should be adjusted 🙂

@aashu2006 aashu2006 requested a review from clairep94 February 23, 2026 06:13
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.

2 participants