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

Allow testEnvironment to be accessed within a test without this #894

Closed
ghost opened this issue Nov 19, 2015 · 3 comments
Closed

Allow testEnvironment to be accessed within a test without this #894

ghost opened this issue Nov 19, 2015 · 3 comments
Assignees
Labels
Category: API Status: Declined Type: Enhancement Type: Meta Seek input from maintainers and contributors.

Comments

@ghost
Copy link

ghost commented Nov 19, 2015

I already made a case for this in rwjblue/ember-qunit#212:

Why? It might be confusing to get assert passed as param and having
to use this to access the context. Making it a param also makes it
more semantic, because you will be naming it for what it is: context.

s/context/environment/

An example:

test('example test', (assert, env) => {
  var subject = env.subject();
  // assert stuff
}); 

I'll open a PR a soon as I've got some confirmation that something like this is OK.

@leobalter
Copy link
Member

As I mentioned there, I wonder if it's better to set env as a property of the assert object.

It's been discussed here and, if I'm not wrong, @gibson042 and @Krinkle have some arguments to avoid an extra parameter. One of them is messing up with any QUnit extensions using a second argument for anything else.

Also, the env could be easily handled, and the arrow function can be set without () for the parameters.

test('example test', assert => {
  var { env } = assert;
  var subject = env.subject();
  // assert stuff
}); 

@gibson042
Copy link
Member

My main concern was keeping our module/test callback signatures similar, but it looks like that ship has sailed (not necessarily in a bad way, though, because module callbacks are invoked synchronously but test callbacks are not—they're different beasts).

I am opposed to extending the assert object (in part because I'd like to eventually support drop-in assertion library replacement), and don't think we should do anything with parameters until sorting out the many suggestions that have already been made:

I'm personally in favor of running with the second point and letting moduleObject subsume all necessary functionality, at which point we could add a context parameter to both module and test. Letting test environment continue to be available only from execution context seems appropriate until such a decision is made, but perhaps that could be sooner rather than later?

@Krinkle
Copy link
Member

Krinkle commented Aug 24, 2020

I'm closing this for now, but am open to reconsidering if there's a new use cases or something I may have missed.

I think we should leverage simple and native JavaScript capabilities where possible rather than holding on to a custom concept. In this case, the environment is nothing but a collection of values or methods that you can share between one or more tests, which are handled in the module options and/or through before/after hooks on a per-test or per-module basis.

These before/after hooks naturally have access to the same lexical scope as the tests, and thus there is no need to have a custom environment object. As such this is imho already fixed, and similar to what Mocha encourages its users to do.

Aan example for how to use this at #923 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: API Status: Declined Type: Enhancement Type: Meta Seek input from maintainers and contributors.
Development

No branches or pull requests

3 participants