Skip to content

Dashboard shows tests for review as modified when they are not modified #23517

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

Closed
mike-plummer opened this issue Aug 23, 2022 · 5 comments · Fixed by #24217
Closed

Dashboard shows tests for review as modified when they are not modified #23517

mike-plummer opened this issue Aug 23, 2022 · 5 comments · Fixed by #24217
Assignees

Comments

@mike-plummer
Copy link
Contributor

mike-plummer commented Aug 23, 2022

Current behavior

Reported internally, slack discussion here: https://cypressio.slack.com/archives/C9KG9TQBX/p1660156177204799
Corresponding CLOUD issue here: https://cypress-io.atlassian.net/browse/CLOUD-820

Appears in this example that the test runner is skipping some tests when run on Firefox but not skipping on Chrome, but test runner is not reporting the skipped tests to the dashboard. Dashboard does not consider browser when determining modified state so the remove/re-add behavior is being treated as a modification

Desired behavior

Test runner should send all tests to the dashboard even if they're skipped

Test code to reproduce

See slack conversation

Cypress Version

10.6.0

Node version

16.14.2

Operating System

macOS 12.5.1

Debug Logs

No response

Other

No response

@rockhold
Copy link

rockhold commented Sep 6, 2022

Some conversation about this with Brian, shared here for reference when it gets picked up:

Once we ensure that the App sends in code for tests (whether they are runtime skipped or not) - we also need to verify that the Cloud “displays” them correctly. For instance, would it use a similar pattern to the App where runtime skipped tests are omitted from the UI? Would it adopt a new pattern where those tests are displayed, but a “reason” for why it was skipped is provided? How does the App indicate to the Cloud that a test was skipped for a runtime configuration reason, since today it obviously just “omits” it from the tests sent in. Once we send it in, we need a way to indicate its status.

We will definitely need to send information about the status of the skipped test from the App to the Cloud. Beyond that, we'll need to determine how the Cloud displays the results.

@brian-mann
Copy link
Member

Here's the updated and complete version of my notes and suggested process for triaging this based on the assumed root problem:

image

@emilyrohrbough
Copy link
Member

emilyrohrbough commented Sep 29, 2022

Issues:

  • tests with it.only are marked as pending and the test body is not sent, so the dashboard is marking it as modified because body was modified
  • tests skipped due to browser skip (i.e. it('t1', { browser: 'chrome' })) is marked as pending and the test body is not sent, so the dashboard is marking it as modified because body was modified
  • suites skipped for .only is marking all tests as pending and is the test body is not sent for each test int he suite, so the dashboard is marking them as modified because body was modified
  • suites skipped due to browser skip (i.e. it('t1', { browser: 'chrome' })) is marking all tests as pending and is the test body is not sent for each test in the suite AND the test path is not correctly restored to the original name (sent as suite1 (skipped due to browser) > t1 instead of suite1 > t1, so the dashboard is marking them as modified because body and title was modified.

Question: Should we really be sending additional data to provide better insights in the dashboard as to by the test was marked as pending (skipped)?

Next steps: Sync with product to determine the experience we want for users & the requirements to satisfy that experience.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 14, 2022

The code for this is done in cypress-io/cypress#24217, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot removed the stage: needs review The PR code is done & tested, needs review label Oct 14, 2022
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 25, 2022

Released in 10.11.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v10.11.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants