fix(angular): mount cy-root in original location

[mv740]

• Closes Angular CT does not mount component within div[data-cy-root]'s original html element hierarchy

Additional details

Steps to test

Throw a .only on

https://github.com/mv740/cypress/blob/e7879be246f0db6e24901ece225eacc12afb3bde/system-tests/test/component_testing_spec.ts#L147

test with yarn test component_testing

How has the user experience changed?

PR Tasks

• Have tests been added/updated?
• Has a PR for user-facing changes been opened in cypress-documentation?
• Have API changes been updated in the type definitions?

[CLAassistant]

All committers have signed the CLA.

[cypress]

26 flaky tests on run #44429 ↗︎

0

26869

1281

0

26

Details:

fix(angular): mount cy-root in original location
Project: cypress	Commit: 8790e2ae45
Status: Passed	Duration: 18:46 💡
Started: Feb 27, 2023 8:26 PM	Ended: Feb 27, 2023 8:45 PM

  create-from-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test
... > runs generated spec		Screenshot

  specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs		Screenshot

  cypress-in-cypress-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test
Cypress In Cypress CT > default config > redirects to the specs list with error if a spec is not found		Screenshot

  studio/studio.cy.ts • 1 flaky test • app-e2e

View Output Video

Test
Cypress Studio > updates an existing test with a click action		Screenshot

  cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e

View Output Video

Test
Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution		Screenshot

The first 5 flaky specs are shown, see all 15 specs in Cypress Cloud.

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[ZachJW34]

We can also trim up the system-tests added. Scaffolding system tests are expensive so if we can tack this on to an existing test that would be ideal. Plus, the system-test added is based on using a project using a custom-root, adding it to the default would make more sense.

You can remove your system-test changes and add:

context('component-index.html', () => {
    before(() => {
      const cyRoot = document.querySelector(cyRootSelector)!

      expect(cyRoot.parentElement === document.body)
      document.body.innerHTML = `
        <div id="container">
          <div data-cy-root></div>
        </div>
      `
    })

    it('preserves html hierarchy', () => {
      cy.mount(ChildComponent, { componentProperties: { msg: 'Render 1' } })
      cy.contains('Render 1')
      cy.get(cyRootSelector).should('exist').parent().should('have.id', 'container')
      cy.get('#container').should('exist').parent().should('have.prop', 'tagName').should('eq', 'BODY')

      // structure persists after teardown
      cy.mount(ChildComponent, { componentProperties: { msg: 'Render 2' } })
      cy.contains('Render 2')
      cy.get(cyRootSelector).should('exist').parent().should('have.id', 'container')
      cy.get('#container').should('exist').parent().should('have.prop', 'tagName').should('eq', 'BODY')
    })
  })

to system-tests/project-fixtures/angular/src/app/mount.cy.ts. This will also run across all versions of angular for added coverage, and since it's already part of an existing scaffold it will be fast.

[ZachJW34]

Suggested change

	if (parentElement && parentElement?.tagName !== 'HTML') {
	parentElement.appendChild(rootElement)
	} else {
	document.body.appendChild(rootElement)
	}

The true fix here is the removal of document.body.appendChild(rootElement), we don't need this extra logic since rootElement is already guaranteed to be in the DOM (getContainerEl() uses querySelector, so if it wasn't in the DOM it will throw)

[mv740]

Oh, I didn't see the mount.cy.ts. You are right, it is a much better idea to inject the different HTML during the test.

I will update my branch based on your suggestion

[lmiller1990]

Will give @jordanpowell88 a chance to review before merging, but this looks good to me.