Skip to content
This repository has been archived by the owner on Mar 25, 2022. It is now read-only.

Add commonjs unit testing unit tests #50

Merged
merged 22 commits into from Oct 13, 2017
Merged

Add commonjs unit testing unit tests #50

merged 22 commits into from Oct 13, 2017

Conversation

JakeChampion
Copy link
Collaborator

Fixes #35

@JakeChampion
Copy link
Collaborator Author

2 failing tests from the new test suite:


  1) commonjs unit testing unit tests converted to mocha test deepEqual "" [""]:

      AssertionError: [Function] throws [Function: AssertionError]
      + expected - actual


      at Function.proclaim.throws (lib/proclaim.js:66:13)
      at Context.exports.test deepEqual "" [""] (test/unit/commonjs-assert/program.js:129:19)

  2) commonjs unit testing unit tests converted to mocha test throw AssertionError:

      AssertionError: type
      + expected - actual

      -false
      +true

      at Function.ok (lib/proclaim.js:14:13)
      at Context.exports.test throw AssertionError (test/unit/commonjs-assert/program.js:156:11)

rowanmanning
rowanmanning previously approved these changes Oct 3, 2017
Copy link
Owner

@rowanmanning rowanmanning left a comment

Choose a reason for hiding this comment

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

This looks good, thanks @JakeChampion. Gonna investigate fixing these issues before merging

@JakeChampion JakeChampion dismissed rowanmanning’s stale review October 5, 2017 14:43

I've made a lot of changes since the review

Copy link
Owner

@rowanmanning rowanmanning left a comment

Choose a reason for hiding this comment

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

Few small comments

README.md Outdated

Assert that `actual` is deeply equal to `expected`, as determined by the strict equality operator `===`.

### proclaim.notStrictEqual( actual, expected, [message] )
Copy link
Owner

Choose a reason for hiding this comment

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

Is this supposed to be notDeepStrictEqual?

@@ -381,7 +390,7 @@ If you're opening issues related to these, please mention the version that the i
License
-------

Proclaim is licensed under the [MIT][info-license] license.
Proclaim is licensed under the [MIT][info-license] license.
Copy link
Owner

Choose a reason for hiding this comment

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

Removal of this whitespace messes up README formatting. Could you add back or replace with an actual <br/>? I've been meaning to for ages 😅

@JakeChampion JakeChampion dismissed rowanmanning’s stale review October 12, 2017 10:38

Made the changes requested by the reviewer, please re-review.

@JakeChampion JakeChampion merged commit e8cd5c0 into master Oct 13, 2017
@JakeChampion JakeChampion deleted the assert-tests branch October 13, 2017 10:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants