Freecodecamp: Testing methodology and frameworks

[ojeytonwilliams]

Let's talk about how we should be testing. It's obvious we should be testing more, but we need some guidelines to help ensure that our tests are high quality and that contributors can easily see what they should be doing.

As such I think we should discuss which testing frameworks we should use and where we should use them.

Right now we use:

• Cypress for e2e, which means we use Mocha when writing those tests
• Mocha in test-challenges.js
• Jest everywhere else (I think)
• Enzyme for challenge tests (the tests that run on user submitted code)
• React Testing Library for some component testing
• react-test-renderer for others
• redux-saga-test-plan for one Redux Saga

I think that covers it!

@freeCodeCamp/dev-team does anyone want to chime in with things we should or should not be doing?

[ojeytonwilliams]

I think we should discuss for a bit and then we can put our conclusions in the contribute docs.

[ojeytonwilliams]

Also, how do we want to handle persistent data? For example, it would be good to have tests that ensure that users can submit challenges and projects. We also want confirm that settings changes immediately take effect and survive page reloads. Similarly, if a user starts donating, that should not require a reload before the client updates. All of which would require the DB and the client to be in known states before the tests are run.

Does this mean we should reset the DB before running the test suite? If so, that might be frustrating for a contributor if their local DB kept getting reset. Particularly if they're working on fcc locally, run the e2e tests (for whatever reason) and lose all their work.

If we go the reset route, I think we should connect the tests to a separate DB that we seed each time. Since this has the potential for utter disaster, we'd need to think carefully about how to 100% guarantee that we never touch the wrong DB.

[raisedadead]

We could move over to testing-library for most things:

• Cypress for e2e, which means we use Mocha when writing those tests

TL has @testing-library/cypress

• Mocha in test-challenges.js

Mocha is used quite well, I am biased towards more fast / async - ava

• Jest everywhere else (I think)

Well maintained and good for most things, can live with it.

• Enzyme for challenge tests (the tests that run on user submitted code)

This is a non-trivial one. I believe we would be needing to take this up seprately as the part of editor/challenge test-runner package.

• React Testing Library for some component testing
• react-test-renderer for others

We should switch to @testing-library/react

• redux-saga-test-plan for one Redux Saga

I dont even want to use redux at all anymore. I think if we moved to the latest of the react versions, we could use the useState (see: https://reactjs.org/docs/hooks-state.html) and useHooks which could be more than enough and simplfy our state managment.

We could then simply use the RTL from TL, to test components.

With that we should be able to achive most usecases which are broadly:

Unit testing challenges:

ava is a better alternative to mocha, but I can live with the later.

Unit testing components:

Lets use RTL from TL, please.

E2E testing:

Cypress with CTL from TL.

User code testing:

Techinically user code testing is part of a "product" that we design, so really should be discussed separately as a design decision from the editor perspective.

[ojeytonwilliams]

I'm all in favour of TL. I think it's worth taking the time to migrate.

Unit testing challenges:

ava is a better alternative to mocha, but I can live with the later.

I can't remember the last time we needed to change that code. Maybe shift to ava if the tests end up taking too long and we need to do a big re-write? This could happen with the new project steps and helpers (i.e. more and slower tests).

Enzyme for challenge tests (the tests that run on user submitted code)

This is a non-trivial one. I believe we would be needing to take this up seprately as the part of editor/challenge test-runner package.

The rationale for using Enzyme is that it really lets you dig into the implementation details. While this isn't great for unit testing, it is for testing user code. I brought it up because I was trying cover everything testing related and it would be ideal if we can use the same tools everywhere. Probably not, but maybe someone else has some inspiration.

[Twaha-Rahman]

If we go the reset route, I think we should connect the tests to a separate DB that we seed each time. Since this has the potential for utter disaster, we'd need to think carefully about how to 100% guarantee that we never touch the wrong DB.

I suggest we use MongoMemoryServer for testing. That way we won't change the local DB.

[Twaha-Rahman]

I dont even want to use redux at all anymore. I think if we moved to the latest of the react versions, we could use the useState (see: https://reactjs.org/docs/hooks-state.html) and useHook which could be more than enough and simplfy our state managment.

@raisedadead I do believe that moving over to the Hooks API should simplify the codebase a LOT. A combination of the useState, useContext and useEffect should be more than enough for state management.

Now the question is, are we or when are we going to start refactoring to use these new APIs?

I'm seeing many deprecation warnings currently in a lot of places (Usage of componentWillMount, componentWillUpdate and componentWillReceiveProps are slowly being deprecated). I think it's a good time to start refactoring as future version of React will deprecate some of the APIs the codebase is using. Also, the React team is getting ready to roll out Concurrent Mode in a year.

[ojeytonwilliams]

are we or when are we going to start refactoring to use these new APIs?

If we start small and refactor one or two components, I think we can start now. That way we can see how much simpler it is in practice.

[Twaha-Rahman]

As Oliver said here, I think we should use Percy for snapshot/visual testing. Just saying it here to keep it all in one place.

@raisedadead Any thoughts?

[raisedadead]

I clarified in the other thread on the original comment.

[ojeytonwilliams]

I should have replied here in the first place:

https://www.browserstack.com/open-source

It looks like they're happy to support OS projects, so we should be able to work something out.

[ahmadabdolsaheb]

A quick update on getting rid of redux. Although hooks and context could replace redux's state management, we use redux actions for handling impure functions as well.

We could useEffect for some of those functions; however, we should consider sticking with a library to manage application side effects such as redux-sage.

The more obvious way forward would be to rewrite epics to sagas, rewrite class components to functions, and use redux hooks.

That should considerably clean up our code base a lot and make it more readable.

We could also briefly explain how we use redux and sagas in our guidelines so it would be more obvious to new contributors.

[raisedadead]

Yup - that would be ideal.

I think current pain point is not just that we simply use redux but the implementations are quite convoluted. There are bits and pieces that are follow varying patterns and then are things in the wild that are just lying there unused and yet confusing enough if you are looking for something.

Here is an instance:

I have no clue how these are triggered - Look useful. Unused.

https://github.com/freeCodeCamp/freeCodeCamp/blob/18d2c92da1798dbcfe6d6b3a365bf5edbfaeb93e/client/src/utils/handled-error.js#L23-L84

And this 😮

https://github.com/freeCodeCamp/freeCodeCamp/blob/18d2c92da1798dbcfe6d6b3a365bf5edbfaeb93e/client/src/utils/reallyWeirdErrorMessage.js#L1-L6

Would you be interested in refactoring some of these a bit and add some approaches that you take in the docs for others to follow?

[ahmadabdolsaheb]

I would love to, and believe SAGA & Redux need an issue of their own at this point.
I will look into best practices and create an issue accordingly, just in case other interested contributors get to work on it before I get a chance.

The handleAPIError belongs to a time where we wanted to have the top news articles show up on learn client. It was used in a shortIdSaga that no longer exist and we could safely remove it from the code base.

'reallyWeirdErrorMessage' is just a user friendly message for unexpected errors. We could probably do better.