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

proposal: Hide Test internals. Expose a delegate object instead. #252

Closed
jamestalmage opened this issue Nov 22, 2015 · 4 comments
Closed
Labels
enhancement new functionality

Comments

@jamestalmage
Copy link
Contributor

We expose instances of Test to the user. It's the t in the following code:

test('title', t => {
  t.is(foo, bar);
});

That has all kinds of internal information on it, including

  • assertCount
  • planCount
  • duration
  • _timeStart
  • planStack
  • etc, etc, etc.

We are already looping over all the public methods and binding them to the test here so users can pass around methods without worrying about this bindings.

I suggest we copy those values (perhaps being a bit more careful to select only that which we truly want public), to a secondary object that we expose.

@sindresorhus
Copy link
Member

👍 from me. @vdemedes ?

@sindresorhus sindresorhus added the enhancement new functionality label Nov 23, 2015
@vadimdemedes
Copy link
Contributor

What if we just made them non-enumerable? So that we won't need to use this.options.assertCount instead of this.assertCount inside lib/test.js itself.

@jamestalmage
Copy link
Contributor Author

We would not need to do that. We would create a different object, and put methods on it that were bound to the test instance test:

Test.prototype._createPublicAPI = function () {
  var api = {};
  var self = this;
  listOfPublicMethods.forEach(function (methodName) {
    api[methodName] = self[methodName].bind(self);
  });
};

Or we could use delegates.

The problem with non-enumerable is that they can still be clobbered. I am working on #25 (registering custom assertion methods) - and we don't want to have to force them to avoid our internals as they name their method.

@vadimdemedes
Copy link
Contributor

Oh, I get it now. delegates usage would be really good, love this module.

jamestalmage added a commit to jamestalmage/ava that referenced this issue Nov 30, 2015
jamestalmage added a commit to jamestalmage/ava that referenced this issue Nov 30, 2015
jamestalmage added a commit to jamestalmage/ava that referenced this issue Nov 30, 2015
jamestalmage added a commit to jamestalmage/ava that referenced this issue Nov 30, 2015
jamestalmage added a commit to jamestalmage/ava that referenced this issue Nov 30, 2015
jamestalmage added a commit that referenced this issue Nov 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new functionality
Projects
None yet
Development

No branches or pull requests

3 participants