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

Remove skip on Auth tests now that jsdom bug is fixed #357

Merged
merged 9 commits into from
Apr 27, 2021

Conversation

jhuleatt
Copy link
Collaborator

@jhuleatt jhuleatt commented Apr 7, 2021

jsdom/jsdom#3073 has been merged and released in jsdom 16.5.2! This means we don't need to skip the auth tests any more!

@google-cla google-cla bot added the cla: yes label Apr 7, 2021
@jhuleatt
Copy link
Collaborator Author

jhuleatt commented Apr 7, 2021

update: this won't be ready until a series of dependencies update their jsdom version to 16.5.2. Dependency chain after running yarn why jsdom:

tsdx#jest#@jest#core#jest-config#jest-environment-jsdom

@jhuleatt jhuleatt marked this pull request as draft April 12, 2021 17:05
@jhuleatt
Copy link
Collaborator Author

This is blocked on TSDX updating from Jest 25 to 26: jaredpalmer/tsdx#1003

@jhuleatt jhuleatt changed the title Remove skip on Auth tests! Remove skip on Auth tests now that jsdom bug is fixed Apr 22, 2021
Version 25 of jest uses a version of jsdom that has a bug that prevents it from talking to the Auth emulator.
@jhuleatt
Copy link
Collaborator Author

trying out Yarn resolutions as suggested in jaredpalmer/tsdx#1016 (comment)

@jhuleatt
Copy link
Collaborator Author

Using the new Jest/jsdom fixed the Auth emulator tests, but broke the Firestore emulator tests. Trying the workaround in firebase/firebase-js-sdk#3096 (comment) to see if that helps

@jhuleatt
Copy link
Collaborator Author

@jamesdaniels any idea why we'd get ReferenceError: globalThis is not defined only on Node 10?

@jhuleatt
Copy link
Collaborator Author

jhuleatt commented Apr 27, 2021

Hm, I tried a potential fix, but ended up breaking one of the Node 12 tests too

EDIT: seems like a false alarm, error in Node 12 was a grpc issue (so probably a flake). Node 10 still has the globalThis error

@jhuleatt
Copy link
Collaborator Author

Seems like this is a jsdom bug with Node 10: jsdom/jsdom#2795 (comment)

@jamesdaniels would you be cool with dropping Node 10 from our test matrix given it is EOL this week?

@jhuleatt jhuleatt marked this pull request as ready for review April 27, 2021 19:21
@jhuleatt
Copy link
Collaborator Author

admin-merging this since this only affects tests (which are passing) and generated docs

@jhuleatt jhuleatt merged commit ecb4996 into main Apr 27, 2021
@jhuleatt jhuleatt deleted the jhuleatt-add-auth-tests branch April 27, 2021 19:25
@FirebaseExtended FirebaseExtended locked and limited conversation to collaborators May 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant