Skip to content

Commit

Permalink
fix: do not magically set user preferences for auto-scroll (#26102)
Browse files Browse the repository at this point in the history 
* fix: Do not magically mutate the user preferences when auto scroll behavior is disabled when a user scrolls or clicks inside the reporter or opens user preferences. Users must explicitly toggle auto-scroll on/off inside the user preferences menu and any implicit behavior will be overidden when this configuration value is set.

* [run ci]

* chore: add CHANGELOG entry

* Update cli/CHANGELOG.md

Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>

* chore: move bugfix to 12.8.1 in changelog

* chore: remove issue 25084 from fix

* chore: rename autoScrollingUnderUserPreference to autoScrollingUserPref

* empty

* Update packages/reporter/src/lib/events.ts

Co-authored-by: Mark Noonan <mark@cypress.io>

* test: add cy-in-cy regression test to make sure auto scroll is not implicitly turned off

* chore: update changelog entry to include #26113

* Update system-tests/projects/cypress-in-cypress/cypress/e2e/dom-content-scrollable-commands.spec.js

Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>

* chore: update the specs list app test since we have added a new cy-in-cy test

* chore: update the specChange-subscription tests

* Update CHANGELOG.md

* chore: update missed assertion update in spec change subscription

---------

Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Co-authored-by: Mark Noonan <mark@cypress.io>
  • Loading branch information
@AtofStryker @emilyrohrbough @marktnoonan
3 people authored Mar 15, 2023
1 parent 91072e0 commit ef430e9
Show file tree
Hide file tree
Showing 11 changed files with 150 additions and 54 deletions.
13 changes: 12 additions & 1 deletion cli/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
<!-- See the ../guides/writing-the-cypress-changelog.md for details on writing the changelog. -->
## 12.8.1

_Released 03/15/2023 (PENDING)_

**Bugfixes:**

- Fixed a regression in Cypress [10](https://docs.cypress.io/guides/references/changelog#10-0-0) where the reporter auto-scroll configuration inside user preferences was unintentionally being toggled off. User's must now explicitly enable/disable auto-scroll under user preferences, which is enabled by default. Fixes [#24171](https://github.com/cypress-io/cypress/issues/24171) and [#26113](https://github.com/cypress-io/cypress/issues/26113).

**Dependency Updates:**

- Upgraded [`ejs`](https://www.npmjs.com/package/ejs) from `3.1.6` to `3.1.8` to address this [CVE-2022-29078](https://github.com/advisories/GHSA-phwq-j96m-2c2q) NVD security vulnerability. Addressed in [#25279](https://github.com/cypress-io/cypress/pull/25279).

## 12.8.0

_Released 03/14/2023_
Expand Down Expand Up @@ -28,7 +40,6 @@ _Released 03/14/2023_

**Dependency Updates:**

- Upgraded [`ejs`](https://www.npmjs.com/package/ejs) from `3.1.6` to `3.1.8` to address this [CVE-2022-29078](https://github.com/advisories/GHSA-phwq-j96m-2c2q) NVD security vulnerability. Addressed in [#25279](https://github.com/cypress-io/cypress/pull/25279).
- Upgraded [`mocha-junit-reporter`](https://www.npmjs.com/package/mocha-junit-reporter) from `2.1.0` to `2.2.0` to be able to use [new placeholders](https://github.com/michaelleeallen/mocha-junit-reporter/pull/163) such as `[suiteFilename]` or `[suiteName]` when defining the test report name. Addressed in [#25922](https://github.com/cypress-io/cypress/pull/25922).

## 12.7.0
Expand Down
86 changes: 61 additions & 25 deletions packages/app/cypress/e2e/reporter_header.cy.ts
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
describe('Reporter Header', () => {
beforeEach(() => {
cy.scaffoldProject('cypress-in-cypress')
cy.openProject('cypress-in-cypress')
cy.startAppServer()
cy.visitApp()
cy.contains('dom-content.spec').click()
cy.waitForSpecToFinish()
})

context('Specs Shortcut', () => {
beforeEach(() => {
cy.scaffoldProject('cypress-in-cypress')
cy.openProject('cypress-in-cypress')
cy.startAppServer()
cy.visitApp()
cy.contains('dom-content.spec').click()
cy.waitForSpecToFinish()
})

it('selects the correct spec in the Specs List', () => {
cy.get('body').type('f')

cy.get('[data-selected-spec="true"]').should('contain', 'dom-content').should('have.length', '1')
cy.get('[data-selected-spec="false"]').should('have.length', '27')
cy.get('[data-selected-spec="false"]').should('have.length', '28')
})

// TODO: Reenable as part of https://github.com/cypress-io/cypress/issues/23902
Expand All @@ -38,26 +38,62 @@ describe('Reporter Header', () => {
})

context('Testing Preferences', () => {
it('clicking the down arrow will open a panel showing Testing Preferences', () => {
cy.get('.testing-preferences-toggle').trigger('mouseover')
cy.get('.cy-tooltip').should('have.text', 'Open Testing Preferences')

cy.get('.testing-preferences').should('not.exist')
cy.get('.testing-preferences-toggle').should('not.have.class', 'open').click()
cy.get('.testing-preferences-toggle').should('have.class', 'open')
cy.get('.testing-preferences').should('be.visible')
cy.get('.testing-preferences-toggle').click()
cy.get('.testing-preferences-toggle').should('not.have.class', 'open')
cy.get('.testing-preferences').should('not.exist')
const switchSelector = '[data-cy=auto-scroll-switch]'

context('preferences menu', () => {
beforeEach(() => {
cy.scaffoldProject('cypress-in-cypress')
cy.openProject('cypress-in-cypress')
cy.startAppServer()
cy.visitApp()
cy.contains('dom-content.spec').click()
cy.waitForSpecToFinish()
})

it('clicking the down arrow will open a panel showing Testing Preferences', () => {
cy.get('.testing-preferences-toggle').trigger('mouseover')
cy.get('.cy-tooltip').should('have.text', 'Open Testing Preferences')

cy.get('.testing-preferences').should('not.exist')
cy.get('.testing-preferences-toggle').should('not.have.class', 'open').click()
cy.get('.testing-preferences-toggle').should('have.class', 'open')
cy.get('.testing-preferences').should('be.visible')
cy.get('.testing-preferences-toggle').click()
cy.get('.testing-preferences-toggle').should('not.have.class', 'open')
cy.get('.testing-preferences').should('not.exist')
})

it('will show a toggle beside the auto-scrolling option', () => {
cy.get('.testing-preferences-toggle').click()
cy.get(switchSelector).invoke('attr', 'aria-checked').should('eq', 'true')
cy.get(switchSelector).click()
cy.get(switchSelector).invoke('attr', 'aria-checked').should('eq', 'false')
})
})

it('will show a toggle beside the auto-scrolling option', () => {
const switchSelector = '[data-cy=auto-scroll-switch]'
it('does NOT toggle off the user preferences auto-scroll if auto-scroll is temporarily disabled', () => {
cy.scaffoldProject('cypress-in-cypress')
cy.openProject('cypress-in-cypress')
cy.startAppServer()
cy.visitApp()
cy.contains('dom-content-scrollable-commands.spec').click()

// wait for the test to scroll all the way to the bottom
cy.get(':contains("checks for list items to exist - iteration #25") + :contains("passed")', {
timeout: 20000,
}).should('exist')

// then, use the runnable container to fire the scroll events that previously would override and sync the preference config
cy.get('[data-cy="reporter-panel"] .reporter > header + .container').its('0').then(($runnableContainer) => {
// scroll the container to the top.
// fire multiple scroll events so our scroller component believes the scroll came from an actual user.
[...Array(10).keys()].forEach(() => {
$runnableContainer.dispatchEvent(new CustomEvent('scroll'))
})
})

cy.get('.testing-preferences-toggle').click()
cy.get(switchSelector).invoke('attr', 'aria-checked').should('eq', 'true')
cy.get(switchSelector).click()
cy.get(switchSelector).invoke('attr', 'aria-checked').should('eq', 'false')
})
})
})
14 changes: 7 additions & 7 deletions packages/app/cypress/e2e/specs_list_e2e.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,21 +144,21 @@ describe('App: Spec List (E2E)', () => {

it('displays only matching spec', function () {
cy.get('button')
.contains('23 matches')
.contains('24 matches')
.should('not.contain.text', 'of')

clearSearchAndType('content')
cy.findAllByTestId('spec-item')
.should('have.length', 2)
.should('have.length', 3)
.and('contain', 'dom-content.spec.js')

cy.get('button').contains('2 of 23 matches')
cy.get('button').contains('3 of 24 matches')

cy.findByLabelText('Search specs').clear().type('asdf')
cy.findAllByTestId('spec-item')
.should('have.length', 0)

cy.get('button').contains('0 of 23 matches')
cy.get('button').contains('0 of 24 matches')
})

it('only shows matching folders', () => {
Expand Down Expand Up @@ -209,7 +209,7 @@ describe('App: Spec List (E2E)', () => {
cy.findByLabelText('Search specs')
.should('have.value', '')

cy.get('button').contains('23 matches')
cy.get('button').contains('24 matches')
})

it('clears the filter if the user presses ESC key', function () {
Expand All @@ -218,7 +218,7 @@ describe('App: Spec List (E2E)', () => {

cy.get('@searchField').should('have.value', '')

cy.get('button').contains('23 matches')
cy.get('button').contains('24 matches')
})

it('shows empty message if no results', function () {
Expand All @@ -234,7 +234,7 @@ describe('App: Spec List (E2E)', () => {
cy.findByText('Clear search').click()
cy.focused().should('have.id', 'spec-filter')

cy.get('button').contains('23 matches')
cy.get('button').contains('24 matches')
})

it('normalizes directory path separators for Windows', function () {
Expand Down
32 changes: 19 additions & 13 deletions packages/app/cypress/e2e/subscriptions/specChange-subscription.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ describe('specChange subscription', () => {
getPathForPlatform('cypress/e2e/blank-contents.spec.js'),
getPathForPlatform('cypress/e2e/dom-container.spec.js'),
getPathForPlatform('cypress/e2e/dom-content.spec.js'),
getPathForPlatform('cypress/e2e/dom-content-scrollable-commands.spec.js'),
getPathForPlatform('cypress/e2e/dom-list.spec.js'),
getPathForPlatform('cypress/e2e/withFailure.spec.js'),
getPathForPlatform('cypress/e2e/withWait.spec.js'),
Expand Down Expand Up @@ -106,6 +107,7 @@ describe('specChange subscription', () => {
getPathForPlatform('cypress/e2e/blank-contents.spec.js'),
getPathForPlatform('cypress/e2e/dom-container.spec.js'),
getPathForPlatform('cypress/e2e/dom-content.spec.js'),
getPathForPlatform('cypress/e2e/dom-content-scrollable-commands.spec.js'),
getPathForPlatform('cypress/e2e/withFailure.spec.js'),
getPathForPlatform('cypress/e2e/withWait.spec.js'),
getPathForPlatform('cypress/e2e/123.spec.js'),
Expand Down Expand Up @@ -171,9 +173,10 @@ e2e: {
})

cy.get('[data-cy="spec-item-link"]', { timeout: 7500 })
.should('have.length', 2)
.should('have.length', 3)
.should('contain', 'dom-container.spec.js')
.should('contain', 'dom-content.spec.js')
.should('contain', 'dom-content-scrollable-commands.spec.js')
})
})

Expand All @@ -185,7 +188,7 @@ e2e: {

cy.get('body').type('f')
cy.get('[data-cy="spec-file-item"]')
.should('have.length', 23)
.should('have.length', 24)
.should('contain', 'blank-contents.spec.js')
.should('contain', 'dom-container.spec.js')
.should('contain', 'dom-content.spec.js')
Expand All @@ -196,7 +199,7 @@ e2e: {
}, { path: getPathForPlatform('cypress/e2e/new-file.spec.js') })

cy.get('[data-cy="spec-file-item"]')
.should('have.length', 24)
.should('have.length', 25)
.should('contain', 'blank-contents.spec.js')
.should('contain', 'dom-container.spec.js')
.should('contain', 'dom-content.spec.js')
Expand All @@ -211,7 +214,7 @@ e2e: {

cy.get('body').type('f')
cy.get('[data-cy="spec-file-item"]')
.should('have.length', 23)
.should('have.length', 24)
.should('contain', 'blank-contents.spec.js')
.should('contain', 'dom-container.spec.js')
.should('contain', 'dom-content.spec.js')
Expand All @@ -222,7 +225,7 @@ e2e: {
}, { path: getPathForPlatform('cypress/e2e/dom-list.spec.js') })

cy.get('[data-cy="spec-file-item"]')
.should('have.length', 22)
.should('have.length', 23)
.should('contain', 'blank-contents.spec.js')
.should('contain', 'dom-container.spec.js')
.should('contain', 'dom-content.spec.js')
Expand All @@ -241,6 +244,7 @@ e2e: {
getPathForPlatform('cypress/e2e/blank-contents.spec.js'),
getPathForPlatform('cypress/e2e/dom-container.spec.js'),
getPathForPlatform('cypress/e2e/dom-list.spec.js'),
getPathForPlatform('cypress/e2e/dom-content-scrollable-commands.spec.js'),
getPathForPlatform('cypress/e2e/withFailure.spec.js'),
getPathForPlatform('cypress/e2e/withWait.spec.js'),
getPathForPlatform('cypress/e2e/123.spec.js'),
Expand Down Expand Up @@ -282,7 +286,7 @@ e2e: {

cy.get('body').type('f')
cy.get('[data-cy="spec-file-item"]')
.should('have.length', 23)
.should('have.length', 24)
.should('contain', 'blank-contents.spec.js')
.should('contain', 'dom-container.spec.js')
.should('contain', 'dom-content.spec.js')
Expand Down Expand Up @@ -310,9 +314,10 @@ e2e: {
})

cy.get('[data-cy="spec-file-item"]', { timeout: 7500 })
.should('have.length', 2)
.should('have.length', 3)
.should('contain', 'dom-container.spec.js')
.should('contain', 'dom-content.spec.js')
.should('contain', 'dom-content-scrollable-commands.spec.js')
})
})

Expand All @@ -324,14 +329,14 @@ e2e: {
cy.get('[data-cy="spec-pattern"]').contains('cypress/e2e/**/*.spec.{js,ts}')

cy.get('[data-cy="file-match-indicator"]')
.should('contain', '23 matches')
.should('contain', '24 matches')

cy.withCtx(async (ctx, o) => {
await ctx.actions.file.writeFileInProject(o.path, '')
}, { path: getPathForPlatform('cypress/e2e/new-file.spec.js') })

cy.get('[data-cy="file-match-indicator"]')
.should('contain', '24 matches')
.should('contain', '25 matches')
})

it('responds to specChange event for a removed file', () => {
Expand All @@ -341,14 +346,14 @@ e2e: {
cy.get('[data-cy="spec-pattern"]').contains('cypress/e2e/**/*.spec.{js,ts}')

cy.get('[data-cy="file-match-indicator"]')
.should('contain', '23 matches')
.should('contain', '24 matches')

cy.withCtx(async (ctx, o) => {
await ctx.actions.file.removeFileInProject(o.path)
}, { path: getPathForPlatform('cypress/e2e/dom-list.spec.js') })

cy.get('[data-cy="file-match-indicator"]')
.should('contain', '22 matches')
.should('contain', '23 matches')
})

it('handles removing the last file', () => {
Expand All @@ -364,6 +369,7 @@ e2e: {
getPathForPlatform('cypress/e2e/blank-contents.spec.js'),
getPathForPlatform('cypress/e2e/dom-container.spec.js'),
getPathForPlatform('cypress/e2e/dom-content.spec.js'),
getPathForPlatform('cypress/e2e/dom-content-scrollable-commands.spec.js'),
getPathForPlatform('cypress/e2e/withFailure.spec.js'),
getPathForPlatform('cypress/e2e/withWait.spec.js'),
getPathForPlatform('cypress/e2e/123.spec.js'),
Expand Down Expand Up @@ -404,7 +410,7 @@ e2e: {
cy.get('[data-cy="spec-pattern"]').contains('cypress/e2e/**/*.spec.{js,ts}')

cy.get('[data-cy="file-match-indicator"]')
.should('contain', '23 matches')
.should('contain', '24 matches')

cy.withCtx(async (ctx) => {
await ctx.actions.file.writeFileInProject('cypress.config.js',
Expand All @@ -428,7 +434,7 @@ e2e: {
})

cy.get('[data-cy="file-match-indicator"]', { timeout: 7500 })
.should('contain', '2 matches')
.should('contain', '3 matches')
})
})
})
2 changes: 1 addition & 1 deletion packages/reporter/cypress/e2e/unit/events.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ describe('events', () => {
})

it('emits save:state on save:state', () => {
appState.autoScrollingEnabled = false
appState.autoScrollingUserPref = false
appState.isSpecsListOpen = true
events.emit('save:state')
expect(runner.emit).to.have.been.calledWith('save:state', {
Expand Down
22 changes: 22 additions & 0 deletions packages/reporter/src/lib/app-state.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const defaults: DefaultAppState = {
}

class AppState {
@observable autoScrollingUserPref = true
@observable autoScrollingEnabled = true
@observable isSpecsListOpen = false
@observable isPaused = defaults.isPaused
Expand Down Expand Up @@ -69,6 +70,14 @@ class AppState {
this.setAutoScrolling(!this.autoScrollingEnabled)
}

/**
* Toggles the auto-scrolling user preference to true|false. This method should only be called from the
* preferences menu itself.
*/
toggleAutoScrollingUserPref () {
this.setAutoScrollingUserPref(!this.autoScrollingUserPref)
}

toggleSpecList () {
this.isSpecsListOpen = !this.isSpecsListOpen
}
Expand All @@ -88,6 +97,19 @@ class AppState {
}
}

/**
* Sets the auto scroll user preference to true|false.
* When this preference is set, it overrides any temporary auto scrolling behaviors that may be in effect.
* @param {boolean | null | undefined} isEnabled - whether or not auto scroll should be enabled or disabled.
* If not a boolean, this method is a no-op.
*/
setAutoScrollingUserPref (isEnabled?: boolean | null) {
if (isEnabled != null) {
this.autoScrollingUserPref = isEnabled
this.setAutoScrolling(isEnabled)
}
}

setStudioActive (studioActive: boolean) {
this.studioActive = studioActive
}
Expand Down
Loading

5 comments on commit ef430e9

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ef430e9 Mar 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux arm64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.8.1/linux-arm64/develop-ef430e9bbf8025cd358d9211098bdd4d36d3b7b4/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ef430e9 Mar 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin arm64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.8.1/darwin-arm64/develop-ef430e9bbf8025cd358d9211098bdd4d36d3b7b4/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ef430e9 Mar 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.8.1/linux-x64/develop-ef430e9bbf8025cd358d9211098bdd4d36d3b7b4/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ef430e9 Mar 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.8.1/darwin-x64/develop-ef430e9bbf8025cd358d9211098bdd4d36d3b7b4/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ef430e9 Mar 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the win32 x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.8.1/win32-x64/develop-ef430e9bbf8025cd358d9211098bdd4d36d3b7b4/cypress.tgz

Please sign in to comment.