Lib/assert #5301

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
4 participants

No description provided.

kaiquewdev added some commits Apr 14, 2013

@kaiquewdev kaiquewdev Assert
Checking the depth correctly
the object.

Added some tests to Object.create,
using the same line of reasoning that was
used in tests of classical inheritance.

setImmediate and clearImmediate were discussed,
for not being present in the global context.
0116ed2
@kaiquewdev kaiquewdev Assert
set and clear immediate go back.
change a test of deep, for a correct assertion.
059e38a

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit kaiquewdev/node@0116ed2 has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Commit kaiquewdev/node@059e38a has the following error(s):

  • Commit message must indicate the subsystem this commit changes

The following commiters were not found in the CLA:

  • Kaique da Silva

Please see CONTRIBUTING.md for more information

kaiquewdev added some commits Apr 14, 2013

hackedy commented Apr 14, 2013

I already proposed this change in #5288. Existing tests in test-assert.js rely on ignoring the prototype value and some even depend on the use of .prototype over .__proto__. The assert api is locked and the idiosyncrasies of deepEqual are here to stay, for better or worse.

There's more discussion over at issue #4523.

Thanks for the warning.

I just did not add the code to compare the prototypes. I did a test pass that was making an assertion incorrect.

@hackedy hackedy commented on the diff Apr 14, 2013

test/simple/test-assert.js
assert.doesNotThrow(makeBlock(a.deepEqual, {a: 4, b: '2'}, {a: 4, b: '2'}));
assert.doesNotThrow(makeBlock(a.deepEqual, [4], ['4']));
assert.throws(makeBlock(a.deepEqual, {a: 4}, {a: 4, b: true}),
a.AssertionError);
-assert.doesNotThrow(makeBlock(a.deepEqual, ['a'], {0: 'a'}));
+assert.throws(makeBlock(a.deepEqual, ['a'], {0: 'a'}),
+ a.AssertionError);
@hackedy

hackedy Apr 14, 2013

🔒 This changes the behavior of the assert library but it's already locked. 🔒

@kaiquewdev

kaiquewdev Apr 14, 2013

Truth ... And as this your proposal? Want to help her?

2013/4/14 Ryan Doenges notifications@github.com

In test/simple/test-assert.js:

assert.doesNotThrow(makeBlock(a.deepEqual, {a: 4, b: '2'}, {a: 4, b: '2'}));
assert.doesNotThrow(makeBlock(a.deepEqual, [4], ['4']));
assert.throws(makeBlock(a.deepEqual, {a: 4}, {a: 4, b: true}),
a.AssertionError);
-assert.doesNotThrow(makeBlock(a.deepEqual, ['a'], {0: 'a'}));
+assert.throws(makeBlock(a.deepEqual, ['a'], {0: 'a'}),

  •          a.AssertionError);
    

[image: 🔒] This changes the behavior of the assert library but it's
already lockedhttp://nodejs.org/docs/latest/api/assert.html#assert_assert.
[image: 🔒]


Reply to this email directly or view it on GitHubhttps://github.com/joyent/node/pull/5301/files#r3786885
.

@kaiquewdev

kaiquewdev Apr 14, 2013

Or not depending on you at this time to end?

@hackedy

hackedy Apr 14, 2013

I'm sorry, I'm not sure what you mean. My pull request was closed because the API is locked. This pull request probably won't get merged.

isaacs commented Apr 15, 2013

Sorry, as @hackedy mentioned, this part of node's API is locked. See http://nodejs.org/docs/latest/api/documentation.html#documentation_stability_index for more information.

isaacs closed this Apr 15, 2013

kaiquewdev deleted the kaiquewdev:lib/assert branch Apr 17, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment