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

tests: prefer using .toBeUndefined() #1659

Merged
merged 2 commits into from
Jan 9, 2021
Merged

tests: prefer using .toBeUndefined() #1659

merged 2 commits into from
Jan 9, 2021

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Jan 7, 2021

There's one case where tests fail; I added a comment there, but I'm not sure if it's a bug or not.

EDIT: I think it's intended but please confirm so that I revert that part.

@fb55
Copy link
Member

fb55 commented Jan 8, 2021

The string value is intended, but wrong. jQuery does stringily values here, but undefined as a value is still removed.

Let's keep the test intact here, and track this in a separate issue?

@5saviahv
Copy link
Contributor

5saviahv commented Jan 8, 2021

maybe add TODO comment for a test

@XhmikosR XhmikosR marked this pull request as ready for review January 9, 2021 05:50
@fb55 fb55 merged commit 5aa4272 into cheeriojs:main Jan 9, 2021
@fb55
Copy link
Member

fb55 commented Jan 9, 2021

Great patch, thanks!

@XhmikosR XhmikosR deleted the toBeUndefined branch January 9, 2021 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants