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.

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(material/core): test environment check not picking up jest #23722

Merged
merged 1 commit into from
Nov 8, 2021

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Oct 10, 2021

In #23636 the test environment check was changed so that it looks for the test objects either on window or global, however it looks like this isn't enough to pick up Jest which isn't published on either.

These changes simplify the setup by looking up the value globally and disabling the type checking error with @ts-ignore. We can't use declare const for it, because it causes issues in g3.

I've also reverted some of the any types that had to be added in #23636.

Fixes #23365.

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 10, 2021
@crisbeto crisbeto force-pushed the 23365/test-env-check-yet-again branch from 3f767f1 to 35fd965 Compare October 10, 2021 10:02
@crisbeto crisbeto marked this pull request as ready for review October 10, 2021 10:10
@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release labels Oct 10, 2021
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

👍 hopefully that works. soo much nicer!

@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Oct 11, 2021
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@wagnermaciel
Copy link
Contributor

@crisbeto This needs to be rebased

@crisbeto crisbeto force-pushed the 23365/test-env-check-yet-again branch from 35fd965 to cdd9415 Compare October 25, 2021 07:07
@crisbeto
Copy link
Member Author

Rebased.

In angular#23636 the test environment check was changed so that it looks for the test objects either on `window` or `global`, however it looks like this isn't enough to pick up Jest which isn't published on either.

These changes simplify the setup by looking up the value globally and disabling the type checkng error with `@ts-ignore`. We can't use `declare const` for it, because it causes issues in g3.

I've also reverted some of the `any` types that had to be added in angular#23636.

Fixes angular#23365.
@crisbeto crisbeto force-pushed the 23365/test-env-check-yet-again branch from cdd9415 to fbe1d24 Compare November 3, 2021 07:51
@mlebarron
Copy link

are we going to get this in a patch release on 12 or will it only be in 13? It continues to litter our testing logs and we won't be able to go to 13 for at least a couple months

@jelbourn
Copy link
Member

jelbourn commented Nov 3, 2021

We'll try to do one last 12.2.x release for this fix after v13

@amysorto amysorto merged commit ff7fd48 into angular:master Nov 8, 2021
amysorto pushed a commit that referenced this pull request Nov 8, 2021
In #23636 the test environment check was changed so that it looks for the test objects either on `window` or `global`, however it looks like this isn't enough to pick up Jest which isn't published on either.

These changes simplify the setup by looking up the value globally and disabling the type checkng error with `@ts-ignore`. We can't use `declare const` for it, because it causes issues in g3.

I've also reverted some of the `any` types that had to be added in #23636.

Fixes #23365.

(cherry picked from commit ff7fd48)
crisbeto added a commit to crisbeto/material2 that referenced this pull request Nov 8, 2021
…ar#23722)

In angular#23636 the test environment check was changed so that it looks for the test objects either on `window` or `global`, however it looks like this isn't enough to pick up Jest which isn't published on either.

These changes simplify the setup by looking up the value globally and disabling the type checkng error with `@ts-ignore`. We can't use `declare const` for it, because it causes issues in g3.

I've also reverted some of the `any` types that had to be added in angular#23636.

Fixes angular#23365.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Nov 8, 2021
…ar#23722)

In angular#23636 the test environment check was changed so that it looks for the test objects either on `window` or `global`, however it looks like this isn't enough to pick up Jest which isn't published on either.

These changes simplify the setup by looking up the value globally and disabling the type checkng error with `@ts-ignore`. We can't use `declare const` for it, because it causes issues in g3.

I've also reverted some of the `any` types that had to be added in angular#23636.

Fixes angular#23365.
amysorto pushed a commit that referenced this pull request Nov 9, 2021
… (#23924)

In #23636 the test environment check was changed so that it looks for the test objects either on `window` or `global`, however it looks like this isn't enough to pick up Jest which isn't published on either.

These changes simplify the setup by looking up the value globally and disabling the type checkng error with `@ts-ignore`. We can't use `declare const` for it, because it causes issues in g3.

I've also reverted some of the `any` types that had to be added in #23636.

Fixes #23365.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Sanity checks run in test environement
6 participants