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

add unit tests for runtime/core - scope #94

Merged
merged 3 commits into from
Oct 11, 2018

Conversation

esell
Copy link
Contributor

@esell esell commented Oct 10, 2018

This PR adds unit tests for runtime/core per issue #34.

Copy link
Member

@ziflex ziflex left a comment

Choose a reason for hiding this comment

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

Could you add a test where a child scope gets a value from its parent?

@esell
Copy link
Contributor Author

esell commented Oct 11, 2018

@ziflex, can you give the latest commit a review and see if that tests the scenario you were looking for? I just forked an existing scope that had a set variable and then ran some tests against the forked Scope.


func TestScope(t *testing.T) {
Convey("Should match", t, func() {
rs, cf := core.NewRootScope()
Copy link
Member

@ziflex ziflex Oct 11, 2018

Choose a reason for hiding this comment

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

Looks good.
Let's do the final change.

RootScope - varA
Child1Scope - varB
Child2Scope - empty

Add a check that Child2Scope is able to get a variable from the root scope. (it will test functionality of scope traversing)

Copy link
Member

@ziflex ziflex Oct 11, 2018

Choose a reason for hiding this comment

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

You can just add it as a 2nd Convey suit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the new TestScopeTraversing should cover this situation, let me know your thoughts :)

Copy link
Member

@ziflex ziflex left a comment

Choose a reason for hiding this comment

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

Looks good!

@ziflex ziflex merged commit ad21fa6 into MontFerret:master Oct 11, 2018
@esell esell deleted the runtime-tests branch October 15, 2018 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants