concord-consortium / codap

CODAP (Common Online Data Analysis Platform)
MIT License
94 stars 38 forks source link

188002783 investigate flaky table tests #1374

Closed nstclair-cc closed 1 month ago

nstclair-cc commented 1 month ago

PT-188002783

Description: This pull request addresses several flaky Cypress tests that needed investigation and fixes. The primary issues seemed to be related to finding target elements, likely due to changes in the elements for editing table cells and editing the slider value component.

Summary of Changes:

  1. Add Logs for CODAP Document Load:

    • Added logs in each beforeEach() statement to ensure that the CODAP document loads correctly. This addition follows the discussion in the standup on 7/24.
  2. Fix Table Cell Editing Tests:

    • Adjusted the selectors and interaction methods to ensure that the tests for editing table cells are more reliable.
  3. Enhance Color Spec Tests:

    • The color spec is functioning correctly locally but fails remotely. I suggested some code but commented it out for now and will create a separate PT story to fix the flaky test. This test could be improved if we ensure our HTML includes the correct data-testid attributes.

Details:

Next Steps:


Thank you!

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.74%. Comparing base (4eae95a) to head (9543b33). Report is 34 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1374 +/- ## ========================================== - Coverage 85.83% 85.74% -0.10% ========================================== Files 521 522 +1 Lines 25305 25582 +277 Branches 6897 6534 -363 ========================================== + Hits 21721 21935 +214 - Misses 3308 3493 +185 + Partials 276 154 -122 ``` | [Flag](https://app.codecov.io/gh/concord-consortium/codap/pull/1374/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | Coverage Δ | | |---|---|---| | [cypress](https://app.codecov.io/gh/concord-consortium/codap/pull/1374/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | `72.58% <ø> (-0.33%)` | :arrow_down: | | [jest](https://app.codecov.io/gh/concord-consortium/codap/pull/1374/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | `53.56% <ø> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cypress[bot] commented 1 month ago

1 flaky test on run #3868 ↗︎

0 200 29 0 Flakiness 1

Details:

Merge pull request #1374 from concord-consortium/188002783-investigate-flaky-tab...
Project: codap-v3 Commit: f5d06d0b9c
Status: Passed Duration: 08:57 💡
Started: Jul 26, 2024 10:31 PM Ended: Jul 26, 2024 10:40 PM
Flakiness  cypress/e2e/toolbar.spec.ts • 1 flaky test View Output
Test Artifacts
codap toolbar > will open a slider Test Replay Screenshots

Review all test suite changes for PR #1374 ↗︎

kswenson commented 1 month ago

Also, moving forward we should:

  1. Figure out a way to add that cy.intercept() line to all of the tests (preferably better than copy-paste).
  2. Remove the logging around the initial load assuming this solves the problem.
nstclair-cc commented 1 month ago
  • Figure out a way to add that cy.intercept() line to all of the tests (preferably better than copy-paste).
  • Remove the logging around the initial load assuming this solves the problem.

Thank you @kswenson! I think I'll merge the code for now so that we can fix the flaky tests issue. I also added a separate PT story here for the issues raised: https://www.pivotaltracker.com/story/show/188015800