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

Assert: Introduce strict boolean assertions. #1445

Closed
wants to merge 4 commits into from

Conversation

ventuno
Copy link
Member

@ventuno ventuno commented Jun 6, 2020

Closes #1444.

TODO:

  • docs;
  • naming?

@Krinkle
Copy link
Member

Krinkle commented Jun 20, 2020

Looks good to me. I'll do a more close review of the source code later on, but let's get the docs in meanwhile as well?

The docs use Jekyll and Markdown. See docs/. I hope it guides itself, but let me know if not :)

@ventuno
Copy link
Member Author

ventuno commented Jun 20, 2020

Thanks @Krinkle, just added docs for assert.true and assert.false.

Let me know if you'd like me to make some changes to the documentation for ok/notOk as well.
There are a number of references and usages of assert.ok/assert.notOk all over the documentation so if we decide to discourage its usage those should probably be changed to something else instead. I would suggest we take this up in a separate PR and perhaps discuss this in a new issue first?

@Krinkle
Copy link
Member

Krinkle commented Jun 20, 2020

Yep, my thoughts exactly. Let's tackle that separately. I'll try to review this PR soon :) Thanks!

@Krinkle Krinkle self-assigned this Jun 20, 2020
@Krinkle Krinkle self-requested a review June 20, 2020 22:25
@ventuno
Copy link
Member Author

ventuno commented Jun 21, 2020

Thanks @Krinkle: created #1450.


## `false( state [, message ] )`

A boolean check, inverse of `true()` equivalent to Chais's `assert.isFalse()`, and JUnit's `assertFalse()`. Passes if the first argument is false.
Copy link
Member

Choose a reason for hiding this comment

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

Chai's :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch: fixed.

docs/assert/false.md Outdated Show resolved Hide resolved
src/assert.js Outdated Show resolved Hide resolved
@ventuno ventuno requested a review from Krinkle June 23, 2020 04:32
categories:
- assert
redirect_from:
- "/true/"
Copy link
Member

Choose a reason for hiding this comment

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

Many files contain this for compat with URLs from an older iteration of the API doc site (which was based on WordPress and jquer-wp-content). Not needed for new pages. I'll omit this while landing.


| name | description |
|--------------------|--------------------------------------|
| `state` | Expression being tested |
Copy link
Member

Choose a reason for hiding this comment

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

This is a left-over from ok(), for which in turn I actually don't know where it originates, but in general we call this the actual value. I'll use that here as well when landing the commit.

| name | description |
|--------------------|--------------------------------------|
| state | Expression being tested |
| message (string) | A short description of the assertion |
Copy link
Member

Choose a reason for hiding this comment

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

missing backticks :)

Krinkle pushed a commit that referenced this pull request Jun 25, 2020
@Krinkle Krinkle closed this in 0a0b7b8 Jun 25, 2020
@ventuno ventuno deleted the ftr-1444 branch June 25, 2020 03:12
@ventuno
Copy link
Member Author

ventuno commented Jun 25, 2020

Thanks @Krinkle any idea when these changes will be released?

@Krinkle
Copy link
Member

Krinkle commented Jun 25, 2020

@ventuno I'm aiming for within the next few weeks, in July?

@ventuno
Copy link
Member Author

ventuno commented Jun 25, 2020

Sounds good. Thanks!

@bmish
Copy link
Member

bmish commented Dec 2, 2020

I'd like to add types for these new assertions in @types/qunit: DefinitelyTyped/DefinitelyTyped#49911

@Krinkle
Copy link
Member

Krinkle commented Dec 3, 2020

@bmish Thanks. I've left a review over there, LGTM. Would you be interested in contributing type definitions into QUnit itself?

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.

Introduce strict boolean assertions
3 participants