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

Support test context from befores #838

Merged
merged 4 commits into from Jun 5, 2018

Conversation

@geek
Copy link
Member

geek commented Jun 1, 2018

No description provided.

@geek geek added the feature label Jun 1, 2018
@Marsup

This comment has been minimized.

Copy link
Member

Marsup commented Jun 1, 2018

It's a bit weird that the nested tests don't share the upper context, don't you think ?

@geek

This comment has been minimized.

Copy link
Member Author

geek commented Jun 1, 2018

@Marsup thanks for the feedback, I updated to support the parent context

@Marsup

This comment has been minimized.

Copy link
Member

Marsup commented Jun 1, 2018

I just hope people won't put heavy objects on that context, otherwise the cloning will be very expansive. (ie. I'm not sure the last commit is really necessary)

README.md Outdated
@@ -277,6 +277,23 @@ lab.test('cleanups after test', (flags) => {
});
```

#### `context`

`context` is an object that is passed to before and after functions in addition to tests themselves. The intent is to be able to set properties on `context` inside of a before function that can be used by a test function later. This should help reduce module level variables that are set by `before`/`beforeEach` functions. Tests aren't able to manipulate the context object for other tests.

This comment has been minimized.

Copy link
@cjihrig

cjihrig Jun 1, 2018

Contributor

The before and afters in this paragraph should probably be in backticks. You should probably also mention that contexts are inherited.

@@ -277,7 +279,7 @@ internals.executeExperiments = async function (experiments, state, skip) {

// Sub-experiments

await internals.executeExperiments(experiment.experiments, state, skipExperiment);
await internals.executeExperiments(experiment.experiments, state, skipExperiment, state.currentContext);

This comment has been minimized.

Copy link
@cjihrig

cjihrig Jun 1, 2018

Contributor

Since this is attached to state, and state is already an argument, do you need to add a new argument?

This comment has been minimized.

Copy link
@geek

geek Jun 1, 2018

Author Member

state gets reused, so I want to avoid any confusion

expect(context.second).to.equal(2);
expect(context.third).to.equal(3);
expect(context.fourth).to.equal(4);
expect(context.test).to.not.exist();

This comment has been minimized.

Copy link
@cjihrig

cjihrig Jun 1, 2018

Contributor

I'm not sure about this. If the assumption is that each test has its own context, I'd expect context.test to exist for one of the invocations here. I guess it could be hard to track individual tests across before, beforeEach, the test, after, and afterEach. Maybe this should be documented.

This comment has been minimized.

Copy link
@geek

geek Jun 1, 2018

Author Member

I don't want a test to be able to manipulate the context, so keeping each separate for now... we can definitely change this if we want

@geek

This comment has been minimized.

Copy link
Member Author

geek commented Jun 1, 2018

@Marsup yep, agreed on the clone. I'm planning to use it for very simple objects

@geek geek added this to the 15.5.0 milestone Jun 1, 2018
@geek geek self-assigned this Jun 5, 2018
@geek geek merged commit 126ad57 into hapijs:master Jun 5, 2018
2 checks passed
2 checks passed
Node Security No known vulnerabilities found
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@geek geek deleted the geek:context branch Jun 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.