311-data icon indicating copy to clipboard operation
311-data copied to clipboard

Implement an automated front-end testing framework for React components

Open edwinjue opened this issue 3 years ago • 17 comments

Dependency

  • [x] #1778

Overview

We need to implement automated React components tests so that non-working code does not accidentally get introduced to the production website.

Action Items

  • [x] Update /client/package.json to install jest, react-testing-library and other dependencies
  • [x] Configure jest.config.js
  • [ ] Ensure testing environment can access redux store
  • [ ] Enable code coverage for all components under test
  • [ ] Agree on an appropriate directory structure & organization for test files
  • [ ] Create and run example tests (proof of concept)
  • [ ] Set up a GitHub Action to run tests on every PR to the main branch, blocking merge until failing tests pass
  • [ ] Author a testing README.md to help others get started
  • [ ] Come up with a strategy for generating coverage for major features that need testing
  • [ ] Come up with policy for adding tests along with newly implemented features

Dependency

  • [x] needs a volunteer who is very familiar with javascript testing frameworks
  • [x] need a strategy for implementing testing framework into late-stage development

Resources/Instructions

https://jestjs.io/ https://testing-library.com/

edwinjue avatar Sep 01 '22 05:09 edwinjue

Hey @edwinjue, Do you have an update for us on this issue?

Please update:

  • Progress:
  • Blockers:
  • Availability:
  • ETA:

Thanks!

mc759 avatar Dec 13 '22 03:12 mc759

@ryanfchase What's the status of this ticket?

bberhane avatar Jun 04 '24 00:06 bberhane

I don't think progress has been made on this, it should not be in-progress.

I think this ticket is blocked until we have a member who is very comfortable with Javascript testing frameworks to take the lead on this. We'll need to come up with a strategy for integrating testing into our project. I'm going to move this to icebox under said reasoning.

ryanfchase avatar Jun 04 '24 15:06 ryanfchase

Hi! I've written frontend tests for a couple of my projects:

  • here I wrote unit tests from the beginning
  • here I added integration tests to an existing project

and would like to lead work on this issue. I think 8pt is an appropriate size, and I'm comfortable writing documentation for our testing practices.

Notes

  • Edwin has completed several of the action items above in the linked branch 1334-implement-an-automated-front-end-testing-framework-for-react-components, so I'd start by getting it caught up with the main branch and building upon it from there.
  • Re: test file/folder structure, React Testing Library author Kent C. Dodds recommends (source):
    • collocating unit tests with the file that they test
    • creating an e2e/ directory at the project root, for tests that render entire pages

Questions

  1. Are user flows written out and listed somewhere? This will help us to write e2e (end-to-end) tests.

aqandrew avatar Jun 10 '24 19:06 aqandrew

Are user flows written out and listed somewhere? This will help us to write e2e (end-to-end) tests.

@aqandrew we have a variety of flows documented, but some are not covered yet. Check out the Figma page here.

ryanfchase avatar Jun 13 '24 00:06 ryanfchase

@aqandrew I think we're good to open this ticket up to be worked on. I approve of the action items, and I've added action items for me to create the testing Epic and integration test ticket template.

If you agree, please do the following...

  • unassign
  • remove draft label
  • add ready for prioritization label

ryanfchase avatar Jun 13 '24 23:06 ryanfchase

@ryanfchase Sounds good! Just a couple more notes worth mentioning--want to leave a paper trail of this info:

  • [x] We should add another action item for setting up CI (continuous integration) for tests. What this looks like is configuring a GitHub Action to run tests on every PR made to the main branch. Then merging would be blocked until additional commits are pushed to make any failing tests pass. @Skydodle is on board with this idea too

  • Testing a canvas UI poses a challenge because the typical workflow for a frontend test is arrange, act, assert. In practice, this looks like:

    1. Render a component/page.
    2. Find and interact with rendered elements (e.g., click a button)
    3. Make assertions about the DOM state after those interactions (e.g., expect a notification element to be visible)

    Much of the core functionality of the 311 Data map involves making updates to a canvas element (i.e., changes in state won’t be reflected in the DOM). This is similar to Excalidraw, which has implemented many UI tests (example). So I think that exposing the Redux store / map state to the testing environment will be especially crucial.

aqandrew avatar Jun 14 '24 16:06 aqandrew

We should add another action item for setting up CI (continuous integration) for tests. What this looks like is configuring a GitHub Action to run tests on every PR made to the main branch. Then merging would be blocked until additional commits are pushed to make any failing tests pass. @Skydodle is on board with this idea too

Sounds good. Absolutely, we should pursue this. Go ahead and modify the action items to include this.

I’m thinking about starting an issue to jot down every time someone provides information that should go in a wiki. I am making a mental note of your comment, and then adding to my todo list from Wednesday to create a wiki page compilation ticket.

ryanfchase avatar Jun 14 '24 17:06 ryanfchase

I've modified the milestone to reflect the correct level of prioritization and importance. This ticket is now available to be picked up.

ryanfchase avatar Jun 14 '24 17:06 ryanfchase

I’m thinking about starting an issue to jot down every time someone provides information that should go in a wiki.

Good idea! Though I think we can live without this particular info going into the wiki. The approach I want to take is the same as what's currently documented in the latter half of contributing.md:

https://github.com/hackforla/311-data/blob/a16ce587e937e5b6a52579e6f8ebecc5e628c92e/contributing.md?plain=1#L15-L23

(^This actually answers a question I'd had about the All PR Status checks are successful checklist item in the PR template. I guess status checks were not yet implemented when the PR template was created)

aqandrew avatar Jun 14 '24 18:06 aqandrew

@aqandrew that's true, we did have this in our contributing.md. No sweat, we can coordinate on how we approach the wiki -- I'll make sure we cover it at the upcoming engineering meeting.

ryanfchase avatar Jun 14 '24 18:06 ryanfchase

@aqandrew FYI I've actually added an action item to the Testing Epic for implementing the CI you mentioned above. Ofc, you're still free to add a similar action item here, I'll just make a note in the epic. But if you want, we can split up the CI work into its own ticket to make this ticket easier to close: https://github.com/hackforla/311-data/issues/1768

ryanfchase avatar Jun 16 '24 04:06 ryanfchase

Sounds good! I think the CI work should go in its own ticket. Since we'll be telling the CI environment to run test commands in the main branch, it'll be helpful to have the testing code merged to main.

aqandrew avatar Jun 19 '24 20:06 aqandrew

ETA: 7/17/24

aqandrew avatar Jun 19 '24 20:06 aqandrew

Hi @aqandrew,

Please leave a comment with the following items:

  • updated ETA
  • progress from the last week (if applicable)
  • availability for communications during the week

bberhane avatar Jun 26 '24 20:06 bberhane