Skip to content
This repository was archived by the owner on Sep 14, 2024. It is now read-only.

Add a Context class #110

Merged
merged 11 commits into from
Jun 18, 2020
Merged

Add a Context class #110

merged 11 commits into from
Jun 18, 2020

Conversation

MagiMaster
Copy link
Collaborator

Add a Context class to pass along to each test node. Follows #109. Part of #108.

@MagiMaster MagiMaster marked this pull request as ready for review June 3, 2020 20:48
@MagiMaster
Copy link
Collaborator Author

Hmm... This might not be good enough. Because of how beforeAll works (it waits until an it block is found before running), something could modify a parent context after the children are created. Either I need to change beforeAll or context needs to support dynamic parents. (I might need to change beforeAll for other reasons, so I need to think on this a bit.)

@MagiMaster
Copy link
Collaborator Author

Actually, this should be fine. It makes more sense to change how beforeAll works than to try and dig up the right context to pass to it. Especially since it looks like beforeAll and afterAll might not be consistent in when they run at the moment.

Copy link
Contributor

@LPGhatguy LPGhatguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks good. We should add a test case to pin the behavior of a parent context being written to after a child context is created. As written right now, the new parent value won't be available through the child context.

@MagiMaster
Copy link
Collaborator Author

I intentionally left out that test case when I was writing this since that behavior wasn't actually decided yet. I think I'm leaning towards keeping the current behavior though as there should be no cases where you can mutate a parent context while a child is in scope and this is the simpler option.

@LPGhatguy LPGhatguy merged commit 7a4f7b8 into Roblox:master Jun 18, 2020
@MagiMaster MagiMaster deleted the context branch June 18, 2020 19:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants