-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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.
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
feat: support Angular 16.1 #27030
feat: support Angular 16.1 #27030
Conversation
|
2 failed and 29 flaky tests on run #47826 ↗︎
Details:
scaffold-project.cy.ts • 1 failed test • launchpad-e2e
commands/querying/querying.cy.js • 1 failed test • 5x-driver-webkit
commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-electron
commands/waiting.cy.js • 1 flaky test • 5x-driver-electron
cypress/cypress.cy.js • 3 flaky tests • 5x-driver-electron
commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox
e2e/origin/basic_login.cy.ts • 1 flaky test • 5x-driver-firefox
The first 5 flaky specs are shown, see all 16 specs in Cypress Cloud. This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
6fe4440
to
d9a15a7
Compare
d9a15a7
to
6c4d09a
Compare
@jordanpowell88 @lmiller1990 The pipeline didn't run and Circle CI is showing this notice in the workflow: Is there anything I should do to unblock this? Or is this something you can help me with? |
Hi @leosvelperez, I think I need to kick off the pipeline, since you are an external contributor. I'll do that now, and give your code a review. |
@@ -195,6 +208,15 @@ export async function getAngularCliModules (projectRoot: string) { | |||
} | |||
} | |||
|
|||
async function getInstalledPackageVersion (pkgName: string, projectRoot: string): Promise<string> { | |||
const packageJsonPath = require.resolve(`${pkgName}/package.json`, { paths: [projectRoot] }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just had to double check, but this looks fine - some packages do not expose package.json
, but this one does: https://github.com/angular/angular-cli/blob/main/packages/angular_devkit/core/package.json#L23
@@ -165,10 +166,22 @@ export async function getTempDir (): Promise<string> { | |||
} | |||
|
|||
export async function getAngularCliModules (projectRoot: string) { | |||
let angularVersion: string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks reasonable. I'd like to see an End to End testing verifying Angular 16.1 is working - can you add one here? https://github.com/cypress-io/cypress/blob/develop/npm/webpack-dev-server/cypress/e2e/angular.cy.ts#L5
Just follow the existing patterns, you can add a new Angular 16.1 project. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the E2E project for Angular 16.1. It seems you'll need to kick off the pipeline again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing this now.
6c4d09a
to
f9e281c
Compare
f9e281c
to
691b852
Compare
hey @lmiller1990! Thanks for the review! Is there anything I can do to unblock this? I'm unsure if the failing checks are related to my change, but it doesn't look like it. |
system-tests/projects/angular-16.1/src/app/components/signals.component.cy.ts
Outdated
Show resolved
Hide resolved
Looks good now. I'll get this one merged up for you. It'll be in the next release, which will be in 2 weeks (we release every 2 weeks). That'll be early July. |
Got some unrelated flake. Please give me another day, I'll get this green and merged up. |
@lmiller1990 thanks!
Is there any way this could be released earlier than that? Releasing in 2 weeks means Angular projects (both with Angular CLI and NX) using Cypress component testing will be hit with this error. Given the Angular 16.1.0 release was over a week ago, that's a total of 3 weeks where folks can't update or can't create new Angular projects using Cypress component testing. |
268d344
to
392ae5f
Compare
I can surface this internally, we don't generally do off cycle releases for issues outside of major regressions - I cannot commit to this right now, since releasing a new version takes the best part of a day, but maybe we can make it happen. It definitely won't be until next week, though, at the earliest -- doing a release is a bit more involved (more than we'd probably like it to be). Great to see the Angular community is so proactive with fixing/patching things to the latest version! Thank you for this, I'll try to get it merged (again) today. |
I think your branch/container is cursed 👻 I remade it and it passed CI first time: #27106 Let's use that PR, it's ready to go - can you add yourself as a reviewer and approve @leosvelperez ? |
Close in favor of #27106 All commits maintained, should be fine. |
Additional details
In Angular 16.1, some internal files' locations consumed by the Cypress Angular handler changed. This causes Cypress to error with:
For reference, this is the commit in the Angular CLI where the change was made: angular/angular-cli@466d86d
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?