Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement an automated front-end testing framework for React components #1334

Open
6 of 13 tasks
Tracked by #1768
edwinjue opened this issue Sep 1, 2022 · 17 comments · May be fixed by #1873
Open
6 of 13 tasks
Tracked by #1768

Implement an automated front-end testing framework for React components #1334

edwinjue opened this issue Sep 1, 2022 · 17 comments · May be fixed by #1873
Assignees
Labels
Dependency An issue that includes dependencies Feature: Code Health Make our code more readable, testable, and modular Role: Frontend React front end work Size: 8pt Can be done in 31-48 hours Tech Stack: New Testing

Comments

@edwinjue
Copy link
Member

edwinjue commented Sep 1, 2022

Dependency

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

  • Update /client/package.json to install jest, react-testing-library and other dependencies
  • 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

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

Resources/Instructions

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

@edwinjue edwinjue added Role: Frontend React front end work Size: 8pt Can be done in 31-48 hours Testing Feature: Code Health Make our code more readable, testable, and modular labels Sep 1, 2022
@edwinjue edwinjue self-assigned this Sep 1, 2022
edwinjue added a commit that referenced this issue Sep 7, 2022
__tests__ accidentally added. directory should not be included until testing framework is in place #1334
@mc759
Copy link
Member

mc759 commented Dec 13, 2022

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

Please update:

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

Thanks!

@bberhane
Copy link
Member

bberhane commented Jun 4, 2024

@ryanfchase What's the status of this ticket?

@ryanfchase
Copy link
Member

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 ryanfchase added Dependency An issue that includes dependencies draft and removed Dependency An issue that includes dependencies labels Jun 4, 2024
@ExperimentsInHonesty ExperimentsInHonesty moved this to New Issue Approval in P: 311: Project Board Jun 7, 2024
@aqandrew
Copy link
Member

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.

@ryanfchase
Copy link
Member

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
Copy link
Member

@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

@aqandrew
Copy link
Member

aqandrew commented Jun 14, 2024

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

  • 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 aqandrew removed their assignment Jun 14, 2024
@ryanfchase
Copy link
Member

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 ryanfchase moved this from New Issue Approval to Prioritized Backlog (ready to be assigned) in P: 311: Project Board Jun 14, 2024
@ryanfchase
Copy link
Member

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

@aqandrew aqandrew self-assigned this Jun 14, 2024
@aqandrew
Copy link
Member

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:

311-data/contributing.md

Lines 15 to 23 in a16ce58

## Branch Protection/Github Actions (to be implemented)
We use [Github Actions](https://github.com/features/actions) to run our continuous integration (CI). These actions include status checks that run whenever you submit a pull request to main. When you submit a PR, Github will run a set of operations to build and test all or part of the codebase. If any of these steps fail, the pull request will not be allowed to be merged until they are fixed. From the pull request UI you can find the reason an operation may have failed in the status checks section towards the bottom.
If you want to look at our setup, check out the "Actions" tab in Github, as well as the [workflows directory](https://github.com/hackforla/311-data/tree/master/.github/workflows), which contains the code that Github runs when actions are triggered.
In addition to status checks, PR's are required to have at least one reviewer before being merged into `main`.
## Testing (to be implemented)
CI Is driven by tests, they help instill confidence in pull requests because a developer can say "All the status checks pass and my new tests pass so the PR is safe to merge". When contributing new features, it is most ideal to write at least 4 tests targeting your code.

(^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)

@ryanfchase
Copy link
Member

@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
Copy link
Member

@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: #1768

@bberhane bberhane moved this from Prioritized Backlog (ready to be assigned) to In progress in P: 311: Project Board Jun 17, 2024
@aqandrew
Copy link
Member

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
Copy link
Member

ETA: 7/17/24

@bberhane
Copy link
Member

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

@ryanfchase
Copy link
Member

ryanfchase commented Aug 22, 2024

@aqandrew iirc you had switched gears to work on this ticket: #1778

I think we ought to shelve this and move it to the Icebox while we wait for the other ticket to go through -- since we agreed it would be easier to approach once we have Vite

@aqandrew
Copy link
Member

That's correct, thank you for the reminder Ryan!

@aqandrew aqandrew moved this from In progress to Icebox (on hold) in P: 311: Project Board Aug 22, 2024
@aqandrew aqandrew added the Dependency An issue that includes dependencies label Aug 22, 2024
@aqandrew aqandrew linked a pull request Dec 6, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependency An issue that includes dependencies Feature: Code Health Make our code more readable, testable, and modular Role: Frontend React front end work Size: 8pt Can be done in 31-48 hours Tech Stack: New Testing
Projects
Status: Icebox (on hold)
7 participants