Open edwinjue opened 2 years ago
Hey @edwinjue, Do you have an update for us on this issue?
Please update:
Thanks!
@ryanfchase What's the status of this ticket?
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.
Hi! I've written frontend tests for a couple of my projects:
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.
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.e2e/
directory at the project root, for tests that render entire pagesAre 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.
@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...
draft
labelready for prioritization
label@ryanfchase Sounds good! Just a couple more notes worth mentioning--want to leave a paper trail of this info:
Testing a canvas UI poses a challenge because the typical workflow for a frontend test is arrange, act, assert. In practice, this looks like:
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.
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.
I've modified the milestone to reflect the correct level of prioritization and importance. This ticket is now available to be picked up.
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:
(^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 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.
@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
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.
ETA: 7/17/24
Hi @aqandrew,
Please leave a comment with the following items:
@aqandrew iirc you had switched gears to work on this ticket: https://github.com/hackforla/311-data/issues/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
That's correct, thank you for the reminder Ryan!
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
Dependency
Resources/Instructions
https://jestjs.io/ https://testing-library.com/