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.

Sign up for GitHub

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

fix: do not magically set user preferences for auto-scroll #26102

Merged
merged 25 commits into from
Mar 15, 2023
Merged

fix: do not magically set user preferences for auto-scroll #26102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e172e39
fix: Do not magically mutate the user preferences when auto scroll be…
AtofStryker Mar 13, 2023
9f7f205
[run ci]
AtofStryker Mar 13, 2023
0a38681
chore: add CHANGELOG entry
AtofStryker Mar 14, 2023
0ded1eb
Merge branch 'develop' of github.com:cypress-io/cypress into spike_sc…
AtofStryker Mar 14, 2023
14a0957
Update cli/CHANGELOG.md
AtofStryker Mar 14, 2023
41c9154
Merge branch 'develop' of github.com:cypress-io/cypress into spike_sc…
AtofStryker Mar 14, 2023
28c6759
chore: move bugfix to 12.8.1 in changelog
AtofStryker Mar 14, 2023
f821276
chore: remove issue 25084 from fix
AtofStryker Mar 14, 2023
aebfa73
chore: rename autoScrollingUnderUserPreference to autoScrollingUserPref
AtofStryker Mar 14, 2023
371050e
empty
AtofStryker Mar 14, 2023
74e3230
Update packages/reporter/src/lib/events.ts
AtofStryker Mar 14, 2023
19c738b
test: add cy-in-cy regression test to make sure auto scroll is not im…
AtofStryker Mar 14, 2023
d925a92
Merge branch 'spike_scroll' of github.com:cypress-io/cypress into spi…
AtofStryker Mar 14, 2023
b3ac61a
chore: update changelog entry to include #26113
AtofStryker Mar 14, 2023
88da3ed
Merge branch 'develop' into spike_scroll
AtofStryker Mar 15, 2023
cc29faf
Update system-tests/projects/cypress-in-cypress/cypress/e2e/dom-conte…
AtofStryker Mar 15, 2023
9d65937
Merge branch 'develop' into spike_scroll
AtofStryker Mar 15, 2023
e7c19d9
chore: update the specs list app test since we have added a new cy-in…
AtofStryker Mar 15, 2023
fd45631
Merge branch 'develop' of github.com:cypress-io/cypress into spike_sc…
AtofStryker Mar 15, 2023
32c1b5d
Merge branch 'spike_scroll' of github.com:cypress-io/cypress into spi…
AtofStryker Mar 15, 2023
cfea4b8
chore: update the specChange-subscription tests
AtofStryker Mar 15, 2023
07099fd
Merge branch 'develop' into spike_scroll
AtofStryker Mar 15, 2023
9843a91
Update CHANGELOG.md
emilyrohrbough Mar 15, 2023
a1eb8da
chore: update missed assertion update in spec change subscription
AtofStryker Mar 15, 2023
9ad1f6f
Merge branch 'spike_scroll' of github.com:cypress-io/cypress into spi…
AtofStryker Mar 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion cli/CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder if it's work a comment linking to https://github.com/cypress-io/cypress/blob/develop/packages/reporter/src/lib/scroller.ts#L37 to explain why this works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would think so. My only concern is if the file changed and the reference became out of data

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah and thinking about it, if you are doing something that breaks this test, you've almost certainly been in that exact function, probably no need for comment.

$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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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