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: Extend mock logger used in angular bootstrap #25500

Merged
merged 4 commits into from
Jan 19, 2023
Merged

fix: Extend mock logger used in angular bootstrap #25500

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
491ba37
fix: Extend mock logger used in angular bootstrap
mike-plummer Jan 18, 2023
9873fa4
Build actual logger and proxy to debug logger
mike-plummer Jan 19, 2023
6a3c33b
Merge branch 'develop' into mikep/25312-angular-logger
mike-plummer Jan 19, 2023
3c62db1
Fix import of Angular logging dependency
mike-plummer Jan 19, 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
133 changes: 78 additions & 55 deletions npm/webpack-dev-server/cypress/e2e/angular.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// <reference types="cypress" />
/// <reference types="cypress" />
/// <reference path="../support/e2e.ts" />
import type { ProjectFixtureDir } from '@tooling/system-tests/lib/fixtureDirs'

Expand All @@ -12,83 +12,106 @@ for (const project of WEBPACK_REACT) {
continue
}

describe(`Working with ${project}`, () => {
context(project, () => {
beforeEach(() => {
cy.scaffoldProject(project)
cy.openProject(project)
cy.startAppServer('component')
})

it('should mount a passing test', () => {
cy.visitApp()
cy.contains('app.component.cy.ts').click()
cy.waitForSpecToFinish({ passCount: 1 }, 60000)
describe('configuration handling', () => {
if (!['angular-13', 'angular-14'].includes(project)) {
it('should initialize with unsupported browserslist entries', () => {
// Create .browerslistrc that requests support for ES5
// Support was dropped in Angular CLI v15 so this should generate a warning message in that version and beyond
cy.withCtx(async (ctx) => {
await ctx.actions.file.writeFileInProject(
ctx.path.resolve('.browserslistrc'),
'IE 11',
)
})

cy.startAppServer('component')
cy.visitApp()
})
}
})

cy.get('li.command').first().within(() => {
cy.get('.command-method').should('contain', 'mount')
cy.get('.command-message').should('contain', 'AppComponent')
describe('test behaviors', () => {
beforeEach(() => {
cy.startAppServer('component')
})
})

it('should live-reload on src changes', () => {
cy.visitApp()
cy.contains('app.component.cy.ts').click()
cy.waitForSpecToFinish({ passCount: 1 }, 60000)
it('should mount a passing test', () => {
cy.visitApp()
cy.contains('app.component.cy.ts').click()
cy.waitForSpecToFinish({ passCount: 1 }, 60000)

cy.withCtx(async (ctx) => {
await ctx.actions.file.writeFileInProject(
ctx.path.join('src', 'app', 'app.component.html'),
(await ctx.file.readFileInProject(ctx.path.join('src', 'app', 'app.component.html'))).replace('Hello World', 'Hello Cypress'),
)
cy.get('li.command').first().within(() => {
cy.get('.command-method').should('contain', 'mount')
cy.get('.command-message').should('contain', 'AppComponent')
})
})

cy.waitForSpecToFinish({ failCount: 1 }, 60000)
it('should live-reload on src changes', () => {
cy.visitApp()
cy.contains('app.component.cy.ts').click()
cy.waitForSpecToFinish({ passCount: 1 }, 60000)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not your change but woah,60000, does the spec really take that long to execute? 🐢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The angular tests especially (and other frameworks to a lesser extent) take forever to bundle & launch webpack on Windows, so I think this was added to work around that.


cy.withCtx(async (ctx) => {
await ctx.actions.file.writeFileInProject(
ctx.path.join('src', 'app', 'app.component.html'),
(await ctx.file.readFileInProject(ctx.path.join('src', 'app', 'app.component.html'))).replace('Hello World', 'Hello Cypress'),
)
})

cy.waitForSpecToFinish({ failCount: 1 }, 60000)

cy.withCtx(async (ctx) => {
await ctx.actions.file.writeFileInProject(
ctx.path.join('src', 'app', 'app.component.html'),
(await ctx.file.readFileInProject(ctx.path.join('src', 'app', 'app.component.html'))).replace('Hello Cypress', 'Hello World'),
)
cy.withCtx(async (ctx) => {
await ctx.actions.file.writeFileInProject(
ctx.path.join('src', 'app', 'app.component.html'),
(await ctx.file.readFileInProject(ctx.path.join('src', 'app', 'app.component.html'))).replace('Hello Cypress', 'Hello World'),
)
})

cy.waitForSpecToFinish({ passCount: 1 }, 60000)
})

cy.waitForSpecToFinish({ passCount: 1 }, 60000)
})
it('should show compilation errors on src changes', () => {
cy.visitApp()

it('should show compilation errors on src changes', () => {
cy.visitApp()
cy.contains('app.component.cy.ts').click()
cy.waitForSpecToFinish({ passCount: 1 }, 60000)

cy.contains('app.component.cy.ts').click()
cy.waitForSpecToFinish({ passCount: 1 }, 60000)
// Create compilation error
cy.withCtx(async (ctx) => {
const componentFilePath = ctx.path.join('src', 'app', 'app.component.ts')

// Create compilation error
cy.withCtx(async (ctx) => {
const componentFilePath = ctx.path.join('src', 'app', 'app.component.ts')
await ctx.actions.file.writeFileInProject(
componentFilePath,
(await ctx.file.readFileInProject(componentFilePath)).replace('class', 'classaaaaa'),
)
})

await ctx.actions.file.writeFileInProject(
componentFilePath,
(await ctx.file.readFileInProject(componentFilePath)).replace('class', 'classaaaaa'),
)
// The test should fail and the stack trace should appear in the command log
cy.waitForSpecToFinish({ failCount: 1 }, 60000)
cy.contains('The following error originated from your test code, not from Cypress.').should('exist')
cy.get('.test-err-code-frame').should('be.visible')
})

// The test should fail and the stack trace should appear in the command log
cy.waitForSpecToFinish({ failCount: 1 }, 60000)
cy.contains('The following error originated from your test code, not from Cypress.').should('exist')
cy.get('.test-err-code-frame').should('be.visible')
})
// TODO: fix flaky test https://github.com/cypress-io/cypress/issues/23455
it('should detect new spec', { retries: 15 }, () => {
cy.visitApp()

// TODO: fix flaky test https://github.com/cypress-io/cypress/issues/23455
it('should detect new spec', { retries: 15 }, () => {
cy.visitApp()
cy.withCtx(async (ctx) => {
await ctx.actions.file.writeFileInProject(
ctx.path.join('src', 'app', 'new.component.cy.ts'),
await ctx.file.readFileInProject(ctx.path.join('src', 'app', 'app.component.cy.ts')),
)
})

cy.withCtx(async (ctx) => {
await ctx.actions.file.writeFileInProject(
ctx.path.join('src', 'app', 'new.component.cy.ts'),
await ctx.file.readFileInProject(ctx.path.join('src', 'app', 'app.component.cy.ts')),
)
cy.contains('new.component.cy.ts').click()
cy.waitForSpecToFinish({ passCount: 1 }, 60000)
})

cy.contains('new.component.cy.ts').click()
cy.waitForSpecToFinish({ passCount: 1 }, 60000)
})
})
}
27 changes: 19 additions & 8 deletions npm/webpack-dev-server/src/helpers/angularHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import type { PresetHandlerResult, WebpackDevServerConfig } from '../devServer'
import { dynamicAbsoluteImport, dynamicImport } from '../dynamic-import'
import { sourceDefaultWebpackDependencies } from './sourceRelativeWebpackModules'
import debugLib from 'debug'
import type { logging } from '@angular-devkit/core'

const debug = debugLib('cypress:webpack-dev-server:angularHandler')
const debugPrefix = 'cypress:webpack-dev-server:angularHandler'
const debug = debugLib(debugPrefix)

export type BuildOptions = Record<string, any>

Expand Down Expand Up @@ -203,14 +205,23 @@ export async function getAngularJson (projectRoot: string): Promise<AngularJson>
return JSON.parse(angularJson)
}

function buildLogger (name: string = ''): logging.Logger {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems this is only called once (below) and name is never passed, is that required, or can we remove the argument entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually called in two places, one of which is within itself so it's easy to miss. We call it once with no name for the root logger, and the createChild signature accepts a name for child loggers

const debugLogger = debugLib(`${debugPrefix}${name ? `:${name}` : ''}`)

return {
Copy link
Contributor

Choose a reason for hiding this comment

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

Cannot find any docs in Angular about the logger, nice job figuring this out.

Another option if we want to cover all bases might be using a Proxy and pointing everything that's not one of the properties you defined here as a no-op. Then if someone does something exotic like logger.subscribe() it wouldn't error (it also wouldn't do what they are expecting, though).

Will let you make the final call, just throwing another suggestion out there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Took another look based on your suggestion and I ended up finding a better alternative - just build an actual Logger instance (which is really just a tracker for LogEntry items) and build a subscription that forwards everything to the debug logger. No idea why we weren't just building a Logger instance to begin with, but now we're fully compliant with the contract and get inspectability from the debug logger

name,
log: debugLogger,
debug: debugLogger,
info: debugLogger,
warn: debugLogger,
error: debugLogger,
fatal: debugLogger,
createChild: buildLogger,
} as unknown as logging.Logger
}

function createFakeContext (projectRoot: string, defaultProjectConfig: Cypress.AngularDevServerProjectConfig) {
const logger = {
createChild: () => {
return {
warn: () => {},
}
},
}
const logger: logging.LoggerApi = buildLogger()

const context = {
target: {
Expand Down
2 changes: 1 addition & 1 deletion system-tests/project-fixtures/angular/src/app/app.component.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ import { AppComponent } from './app.component'

it('should', () => {
cy.mount(AppComponent)
cy.get('h1').contains('Hello World')
cy.get('h1').contains('Hello World', { timeout: 250 })
})