Implement Assertion constructor with test context #374

Closed
Krinkle opened this Issue Dec 17, 2012 · 3 comments

Comments

Projects
None yet
3 participants
@Krinkle
Owner

Krinkle commented Dec 17, 2012

To fix #331, and generally make our code more scaleable and flexible.

Each test() will be given an instance of Assertion which has an internal reference back to the Test object, so that they can easily be called asynchronously.

In addition to the assertion helpers, it could have the following built-in to the context based system as well:

  • async()
  • done()
  • expect()
  • .not (instance of itself with internal reversal)
@rodneyrehm

This comment has been minimized.

Show comment Hide comment
@rodneyrehm

rodneyrehm Dec 18, 2012

Contributor

I'm not sure why you need to instantiate Assertion for this. Simply map the available assertions to Test.prototype and use this within an assertion to access the test instance. This should be more memory/garbage collector friendly. To a user it might look like:

QUnit.test("foo", function(test) {
  test.equal("foo", "bar", "context, anyone?");
});

you could also run the test's callback function in the scope of the test instance, allowing

QUnit.test("foo", function() {
  this.equal("foo", "bar", "context, anyone?");
});

having the test instance available in an async test would also allow calling test.abort(), should a test determine that any subsequent assertions/tests would fail anyways…

Contributor

rodneyrehm commented Dec 18, 2012

I'm not sure why you need to instantiate Assertion for this. Simply map the available assertions to Test.prototype and use this within an assertion to access the test instance. This should be more memory/garbage collector friendly. To a user it might look like:

QUnit.test("foo", function(test) {
  test.equal("foo", "bar", "context, anyone?");
});

you could also run the test's callback function in the scope of the test instance, allowing

QUnit.test("foo", function() {
  this.equal("foo", "bar", "context, anyone?");
});

having the test instance available in an async test would also allow calling test.abort(), should a test determine that any subsequent assertions/tests would fail anyways…

@Krinkle

This comment has been minimized.

Show comment Hide comment
@Krinkle

Krinkle Dec 18, 2012

Owner

Whether Assertion, Test or TestPublic extending TestInternal is an implementation detail. I'd prefer to have an exposed prototype with only the assertion helpers to avoid conflicts and make it easy to do things right (and harder to do things wrong). But sure, it makes sense to have it as both context and argument, whatever works best. We'll see during implementation.

Owner

Krinkle commented Dec 18, 2012

Whether Assertion, Test or TestPublic extending TestInternal is an implementation detail. I'd prefer to have an exposed prototype with only the assertion helpers to avoid conflicts and make it easy to do things right (and harder to do things wrong). But sure, it makes sense to have it as both context and argument, whatever works best. We'll see during implementation.

@jzaefferer

This comment has been minimized.

Show comment Hide comment
@jzaefferer

jzaefferer May 31, 2014

Owner

#583 demonstrates the issue that this ticket is supposed to address.

Owner

jzaefferer commented May 31, 2014

#583 demonstrates the issue that this ticket is supposed to address.

leobalter added a commit to leobalter/qunit that referenced this issue Jun 24, 2014

Assert: Implement Assertion constructor with test context
Ref #374

- Assert is now an instanced object exclusive to test blocks
- Assert: Uses context test push
  - Ref qunitjs#588 (comment)
- Test: Using related test context instead of config.current

@leobalter leobalter closed this in f2066a2 Jun 24, 2014

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