QA/testing Code Review

[bobbysebolao]

HUGE CONGRATULATIONS ON YOUR PROJECT SO FAR!! Code is looking clean 😎 and you've got some great tests! 🥳

Here's some constructive feedback from me:

1. Testing file uploads

Cypress doesn’t support testing file uploads out of the box (although it’s been heavily requested by the community), but someone has made a plugin called cypress-file-upload that offers a popular workaround.

Since you’re testing image uploads to Cloudinary, you’ll need to think carefully about what test assertion(s) to make to check that an upload attempt has been successful/unsuccessful. For example. if, after a successful upload, a success message displays on the page, a sensible test might assert that this DOM element should be visible. There could be many ways to do it, when you know how you'll implement this feature you will know what is best 😄

Sidenote: Testing file downloads
Just a heads up about testing file downloads, in case you were thinking of testing this, this is another thing that Cypress doesn’t officially support (yet). There is a workaround, but it’s poorly documented and quite fiddly to implement. I would recommend against trying to test file downloads during this sprint (or at least until you've tested everything else that you want to test 😄)

2. Select DOM elements by their data-cy attribute, where possible

Looking at your second it() assertion block in h1.spec.js (lines 10-14), if you were to change the ‘PICTURE STORY’ button text in your LandingPage.js component to something different, it would cause the test to fail at line 12, because cy.contains(‘PICTURE STORY’) would no longer yield a DOM element.

This would be bad because the point of this particular it() block isn’t to check the specific text in that button, the point of the test is to check that clicking the button loads a new page.

Fortunately, you can make Cypress commands less prone to this kind of failure by applying a special data-cy attribute to the DOM elements that you need your tests to target. Cypress recommends this approach because you aren’t likely to change this attribute (and break your tests) during the course of development.

4. Impressive use of Cypress plugins

I was super impressed to see that you’re using the start-server-and-test package in the cy:ci command to instruct Travis to start your app before running your Cypress tests. You’ve clearly read and understood the docs on configuring Cypress with your CI provider. Is it all working? If not, it looks like you’re very close, well done!! 🚀

5. Getting Travis to record your Cypress test runs

I see you’re trying to get Travis to record the videos from tests. One potential issue I spot is on line 12 of .travis.yml:

- npm run cy:ci --record

At the moment I don't think you will be able to access the videos from tests that run on Travis, because Travis doesn’t save the recording anywhere. You need to set up Travis to send the recording to the Cypress Dashboard, where you’ll be able to view it. It’s really straightforward to do this, and the fact that you have set a "projectId” in your cypress.json config file makes me think that you’ve already done it ⭐ . The only thing that’s missing is to add the record key to the above command, like so:

- npm run cy:ci --record --key <add your record key here>

In case you haven’t seen it, here’s the doc that describes how to generate the record key.

[Alexreid95]

Yay!! Thanks a bunch!!!

1. Excited to try the out
   1.5 Testing file downloads. Thanks for mentioning this, it will be at the bottom of our large list of test to do
2. Great, started implementing this, thanks for the tip
3. Yup it works!!
4. Completed this in a recent PR