chore(session): require setup function to use session command

cli/types/cypress.d.ts

@@ -1089,7 +1089,7 @@ declare namespace Cypress {
       *
       * @see https://on.cypress.io/session
       */
-    session(id: string | object, setup?: () => void, options?: SessionOptions): Chainable<null>
+    session(id: string | object, setup: () => void, options?: SessionOptions): Chainable<null>
 
     /**
      * Get the window.document of the page that is currently active.

cli/types/tests/cypress-tests.ts

@@ -922,7 +922,6 @@ namespace CypressTaskTests {
 }
 
 namespace CypressSessionsTests {
-  cy.session('user')
   cy.session('user', () => {})
   cy.session({ name: 'bob' }, () => {})
   cy.session('user', () => {}, {})
@@ -931,6 +930,7 @@ namespace CypressSessionsTests {
   })
 
   cy.session() // $ExpectError
+  cy.session('user') // $ExpectError
   cy.session(null) // $ExpectError
   cy.session('user', () => {}, {
     validate: { foo: true } // $ExpectError

packages/app/cypress/e2e/runner/sessions.ui.cy.ts

@@ -132,71 +132,54 @@ describe('runner/cypress sessions.ui.spec', {
     // cy.percySnapshot() // TODO: restore when Percy CSS is fixed. See https://github.com/cypress-io/cypress/issues/23435
   })
 
-  describe('restores saved session', () => {
-    beforeEach(() => {
-      loadSpec({
-        projectName: 'session-and-origin-e2e-specs',
-        filePath: 'session/restores_saved_session.cy.js',
-        passCount: 5,
-        failCount: 1,
-      })
+  it('restores session as expected', () => {
+    loadSpec({
+      projectName: 'session-and-origin-e2e-specs',
+      filePath: 'session/restores_saved_session.cy.js',
+      passCount: 2,
     })
 
-    it('restores session as expected', () => {
-      cy.get('.test').each(($el, index) => {
-        if (index < 5) { // don't click on failed test
-          cy.wrap($el).click()
-        }
-      })
-
-      cy.log('validate new session was created in first test')
-      cy.get('.test').eq(0).within(() => {
-        validateSessionsInstrumentPanel(['user1'])
-        cy.get('.command-name-session').contains('created')
-      })
-
-      cy.log('validate saved session was used in second test')
-      cy.get('.test').eq(1).within(() => {
-        validateSessionsInstrumentPanel(['user1'])
-        cy.get('.command-name-session')
-        .within(() => {
-          cy.get('.command-expander').first().click()
-          cy.contains('user1')
-          cy.contains('restored')
+    cy.get('.test').each(($el, index) => {
+      if (index < 5) { // don't click on failed test
+        cy.wrap($el).click()
+      }
+    })
 
-          cy.get('.command-name-Clear-page').should('have.length', 1)
+    cy.log('validate new session was created in first test')
+    cy.get('.test').eq(0).within(() => {
+      validateSessionsInstrumentPanel(['user1'])
+      cy.get('.command-name-session').contains('created')
+    })
 
-          cy.contains('Restore saved session')
+    cy.log('validate saved session was used in second test')
+    cy.get('.test').eq(1).within(() => {
+      validateSessionsInstrumentPanel(['user1'])
+      cy.get('.command-name-session')
+      .within(() => {
+        cy.get('.command-expander').first().click()
+        cy.contains('user1')
+        cy.contains('restored')
 
-          cy.contains('Validate session')
-          .closest('.command').as('validateSession')
+        cy.get('.command-name-Clear-page').should('have.length', 1)
 
-          cy.get('@validateSession')
-          .find('.command-expander')
-          .should('not.have.class', 'command-expander-is-open')
-          .click()
+        cy.contains('Restore saved session')
 
-          cy.get('@validateSession')
-          .find('.command-alias')
-          .contains('runValidation')
-        })
+        cy.contains('Validate session')
+        .closest('.command').as('validateSession')
 
-        cy.get('.command-name-session').get('.command-expander').first().click()
+        cy.get('@validateSession')
+        .find('.command-expander')
+        .should('not.have.class', 'command-expander-is-open')
+        .click()
 
-        cy.get('.command').should('have.length', 2)
+        cy.get('@validateSession')
+        .find('.command-alias')
+        .contains('runValidation')
       })
-    })
 
-    // https://github.com/cypress-io/cypress/issues/22381
-    it('ensures sessionid integrity is maintained across tests', () => {
-      cy.contains('test sessionid integrity is maintained').closest('.runnable').should('have.class', 'runnable-failed')
-      cy.get('.test').should('have.length', 6)
+      cy.get('.command-name-session').get('.command-expander').first().click()
 
-      cy.get('.test').eq(2).should('have.class', 'runnable-passed')
-      cy.get('.test').eq(3).should('have.class', 'runnable-passed')
-      cy.get('.test').eq(4).should('have.class', 'runnable-passed')
-      cy.get('.test').eq(5).should('have.class', 'runnable-failed')
-      cy.contains('This session already exists.').should('exist')
+      cy.get('.command').should('have.length', 2)
     })
   })
 

packages/driver/cypress/e2e/commands/sessions/sessions.cy.js

@@ -53,12 +53,11 @@ describe('cy.session', { retries: 0 }, () => {
     })
 
     it('accepts array as id', () => {
-      cy.session('session-id', () => {})
-      cy.session('session-id')
+      cy.session(['session', 'id'], () => {})
     })
 
     it('accepts object as id', () => {
-      cy.session('session-id', () => {})
+      cy.session({ 'session-id': true }, () => {})
     })
 
     // redundant?
@@ -1464,6 +1463,19 @@ describe('cy.session', { retries: 0 }, () => {
       cy.session()
     })
 
+    it('throws when setup function is not provided', function (done) {
+      cy.once('fail', (err) => {
+        expect(lastLog.get('error')).to.eq(err)
+        expect(lastLog.get('state')).to.eq('failed')
+        expect(err.message).to.eq('In order to use `cy.session()`, provide a `setup` as the second argument:\n\n`cy.session(id, setup)`')
+        expect(err.docsUrl).to.eq('https://on.cypress.io/session')
+
+        done()
+      })
+
+      cy.session('some-session')
+    })
+
     it('throws when sessionId argument is not an object', function (done) {
       cy.once('fail', (err) => {
         expect(lastSessionLog).to.eq(lastLog)
@@ -1519,42 +1531,6 @@ describe('cy.session', { retries: 0 }, () => {
       cy.session('some-session', () => {}, { validate: 2 })
     })
 
-    it('throws when setup function is not provided and existing session is not found', function (done) {
-      cy.once('fail', (err) => {
-        expect(lastLog.get('error')).to.eq(err)
-        expect(lastLog.get('state')).to.eq('failed')
-        expect(err.message).to.eq('No session is defined with the name\n  **some-session**\nIn order to use `cy.session()`, provide a `setup` as the second argument:\n\n`cy.session(id, setup)`')
-        expect(err.docsUrl).to.eq('https://on.cypress.io/session')
-
-        done()
-      })
-
-      cy.session('some-session')
-    })
-
-    it('throws when setup function is not provided and global session is registered', function (done) {
-      cy.once('fail', (err) => {
-        expect(lastSessionLog).to.eq(lastLog)
-        expect(lastLog.get('error')).to.eq(err)
-        expect(lastLog.get('state')).to.eq('failed')
-        expect(err.message).to.eq('In order to restore a global `cy.session()`, provide a `setup` as the second argument:\n\n`cy.session(id, setup, { cacheAcrossSpecs: true })`')
-        expect(err.docsUrl).to.eq('https://on.cypress.io/session')
-
-        done()
-      })
-
-      cy.session('some-session-2', () => {}, { cacheAcrossSpecs: true })
-      .then(() => {
-        Cypress.state('activeSessions', {})
-      }).then(async () => {
-        await Cypress.backend('get:session', 'some-session-2').then((sessionDetails) => {
-          Cypress.state('activeSessions', { 'some-session-2': sessionDetails })
-        })
-      })
-
-      cy.session('some-session-2')
-    })
-
     it('throws when multiple session calls with same sessionId but different setup', function (done) {
       cy.once('fail', async (err) => {
         expect(lastSessionLog).to.eq(lastLog)

packages/driver/cypress/e2e/e2e/origin/commands/unsupported_commands.cy.ts

@@ -60,7 +60,7 @@ context('cy.origin unsupported commands', { browser: '!webkit' }, () => {
     })
 
     cy.origin('http://www.foobar.com:3500', () => {
-      cy.session('/foo')
+      cy.session('/foo', () => {})
     })
   })
 })

packages/driver/src/cy/commands/sessions/index.ts

@@ -66,18 +66,16 @@ export default function (Commands, Cypress, cy) {
   })
 
   Commands.addAll({
-    session (id: string | object, setup?: () => void, options: Cypress.SessionOptions = { cacheAcrossSpecs: false }) {
+    session (id: string | object, setup: () => void, options: Cypress.SessionOptions = { cacheAcrossSpecs: false }) {
       throwIfNoSessionSupport()
 
       if (!id || !_.isString(id) && !_.isObject(id)) {
         $errUtils.throwErrByPath('sessions.session.wrongArgId')
       }
 
-      // backup session command so we can set it as codeFrame location for errors later on
-      const sessionCommand = cy.state('current')
-
-      // stringify deterministically if we were given an object
-      id = _.isString(id) ? id : stringifyStable(id)
+      if (!setup || !_.isFunction(setup)) {
+        $errUtils.throwErrByPath('sessions.session.wrongArgSetup')
+      }
 
       if (options) {
         if (!_.isObject(options)) {
@@ -104,55 +102,51 @@ export default function (Commands, Cypress, cy) {
         })
       }
 
+      // backup session command so we can set it as codeFrame location for validation errors later on
+      const sessionCommand = cy.state('current')
+
+      // stringify deterministically if we were given an object
+      id = _.isString(id) ? id : stringifyStable(id)
+
       let session: SessionData = sessionsManager.getActiveSession(id)
       const isRegisteredSessionForSpec = sessionsManager.registeredSessions.has(id)
 
-      if (!setup) {
-        if (!session && !isRegisteredSessionForSpec) {
-          $errUtils.throwErrByPath('sessions.session.not_found', { args: { id } })
+      if (session) {
+        const hasUniqSetupDefinition = session.setup.toString().trim() !== setup.toString().trim()
+        const hasUniqValidateDefinition = (!!session.validate !== !!options.validate) || (!!session.validate && !!options.validate && session.validate.toString().trim() !== options.validate.toString().trim())
+        const hasUniqPersistence = session.cacheAcrossSpecs !== !!options.cacheAcrossSpecs
+
+        if (isRegisteredSessionForSpec && (hasUniqSetupDefinition || hasUniqValidateDefinition || hasUniqPersistence)) {
+          $errUtils.throwErrByPath('sessions.session.duplicateId', {
+            args: {
+              id,
+              hasUniqSetupDefinition,
+              hasUniqValidateDefinition,
+              hasUniqPersistence,
+            },
+          })
         }
 
         if (session.cacheAcrossSpecs && _.isString(session.setup)) {
-          $errUtils.throwErrByPath('sessions.session.missing_global_setup', { args: { id } })
+          session.setup = setup
         }
-      } else {
-        if (session) {
-          const hasUniqSetupDefinition = session.setup.toString().trim() !== setup.toString().trim()
-          const hasUniqValidateDefinition = (!!session.validate !== !!options.validate) || (!!session.validate && !!options.validate && session.validate.toString().trim() !== options.validate.toString().trim())
-          const hasUniqPersistence = session.cacheAcrossSpecs !== !!options.cacheAcrossSpecs
-
-          if (isRegisteredSessionForSpec && (hasUniqSetupDefinition || hasUniqValidateDefinition || hasUniqPersistence)) {
-            $errUtils.throwErrByPath('sessions.session.duplicateId', {
-              args: {
-                id,
-                hasUniqSetupDefinition,
-                hasUniqValidateDefinition,
-                hasUniqPersistence,
-              },
-            })
-          }
-
-          if (session.cacheAcrossSpecs && _.isString(session.setup)) {
-            session.setup = setup
-          }
 
-          if (session.cacheAcrossSpecs && session.validate && _.isString(session.validate)) {
-            session.validate = options.validate
-          }
-        } else {
-          if (isRegisteredSessionForSpec) {
-            $errUtils.throwErrByPath('sessions.session.duplicateId', { args: { id } })
-          }
+        if (session.cacheAcrossSpecs && session.validate && _.isString(session.validate)) {
+          session.validate = options.validate
+        }
+      } else {
+        if (isRegisteredSessionForSpec) {
+          $errUtils.throwErrByPath('sessions.session.duplicateId', { args: { id } })
+        }
 
-          session = sessions.defineSession({
-            id,
-            setup,
-            validate: options.validate,
-            cacheAcrossSpecs: options.cacheAcrossSpecs,
-          })
+        session = sessions.defineSession({
+          id,
+          setup,
+          validate: options.validate,
+          cacheAcrossSpecs: options.cacheAcrossSpecs,
+        })
 
-          sessionsManager.registeredSessions.set(id, true)
-        }
+        sessionsManager.registeredSessions.set(id, true)
       }
 
       function setSessionLogStatus (status: string) {

packages/driver/src/cypress/error_messages.ts

@@ -1716,6 +1716,13 @@ export default {
         `,
         docsUrl: 'https://on.cypress.io/session',
       },
+      wrongArgSetup: {
+        message: stripIndent`
+        In order to use ${cmd('session')}, provide a \`setup\` as the second argument:
+
+        \`cy.session(id, setup)\``,
+        docsUrl: 'https://on.cypress.io/session',
+      },
       wrongArgOptions: {
         message: stripIndent`
         ${cmd('session')} was passed an invalid argument. The optional third argument \`options\` must be an object.
@@ -1735,13 +1742,6 @@ export default {
         `,
         docsUrl: 'https://on.cypress.io/session',
       },
-      missing_global_setup: {
-        message: stripIndent`
-        In order to restore a global ${cmd('session')}, provide a \`setup\` as the second argument:
-
-        \`cy.session(id, setup, { cacheAcrossSpecs: true })\``,
-        docsUrl: 'https://on.cypress.io/session',
-      },
       not_found: {
         message: stripIndent`
         No session is defined with the name

system-tests/projects/session-and-origin-e2e-specs/cypress/e2e/session/restores_saved_session.cy.js

@@ -24,26 +24,3 @@ it('t2', () => {
 
   cy.log('after')
 })
-
-// https://github.com/cypress-io/cypress/issues/22381
-describe('test sessionid integrity is maintained', () => {
-  it('use same session 2x and 2nd does not provide setup', () => {
-    cy.session('session-2', setupFn)
-    cy.session('session-2')
-  })
-
-  it('restore prev session 2x and 2nd does not provide setup', () => {
-    cy.session('session-2', setupFn)
-    cy.session('session-2')
-  })
-
-  it('restore prev session without setup', () => {
-    cy.session('session-2')
-  })
-
-  it('fails when trying to use existing sessionid with diff args', () => {
-    cy.session('session-2', () => {
-    // something else
-    })
-  })
-})