Issue #733 dont use index for keys testcases

[mumbler6]

Attempted to write test cases that supplement issue #733, but have errors from importing and from redux while rendering components.

[harsh183]

Great start! Leaving a few comments for changes

[harsh183]

Might be a good idea to add this to the .gitignore, that way git won't accidently pick up config files.

[harsh183]

nice catch!

[harsh183]

really good use of mock for a randomized ID from an external library. Once you merge this PR, you should definitely add this as an example test file in the Testing wiki

[harsh183]

Looks like jest isn't able to work without a redux state, which is unfortunately wrapped by DvaJS. This might need some research on how to mock but we'll figure this out later.

[harsh183]

This probably needs to be screen.getAllByTestId to fix the error

[harsh183]

If we aren't using these variables anywhere else, might be simpler to just have jest.fn() in props directly similar to your other test files, though this works fine too since the readability is quite clear in both cases.

[harsh183]

@mumbler6 doing the test cases looks like it's getting quite complicated since it's dealing with redux. I think we can put these on the back burner for now to if you want spend more time focusing on something else. Feel free to fix anything from the comments and merge the other PR before wrapping up and starting on another issue.

I can try taking a crack at this in a bit too, I haven't mocked anything redux level yet, so it might take a while to do the initial setup and then future tests will probably go much smoother once we've figured it out.

[angrave]

LGTM. Note in INoteChapter.test.js (line 26)- there was a build failure-

'getAllByTestId' is not defined

    contents.forEach((content) => {
      expect(content).toHaveAttribute('key', `ch-content-chapter-id-1-${test_uuid}}`);
    });```

[angrave]

You will also want to rebase or merge in latest from staging

[mumbler6]

LGTM. Note in INoteChapter.test.js (line 26)- there was a build failure-

'getAllByTestId' is not defined

    contents.forEach((content) => {
      expect(content).toHaveAttribute('key', `ch-content-chapter-id-1-${test_uuid}}`);
    });```

My bad, Harsh left a comment about it before. Hopefully this'll fix it. Though it looks like the test case errors are also part of the build failure.