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

[Test] Fix mocked functions. #1041

Closed
wants to merge 1 commit into from
Closed

Conversation

eloycoto
Copy link
Contributor

If any function is mocked used stub need to be reverted at some point,
doing something like this

before_each(function()
  stub(foo, "validate", function() return false end)
end)

after_each(function()
  foo.validate:revert()
end)

it("Validate stub foo", function()
  assert.same(foo.validate(), false)
end)

In APICast all ngx variables are reset on ngx_helper.lua but some
methods like ngx_variables or backend_client are not reset and the
mock function is in there for all other test.

The test in spec/policy/ngx_variable_spec.lua validates that
ngx_variable.available_context is working correctly, due some other
test can mock this function the test was inestable over the last days.

This commit creates a way to define a cleanup for mocked functions that
are not part of the ngx class.

If any function is mocked used stub need to be reverted at some point,
doing something like this

```
before_each(function()
  stub(foo, "validate", function() return false end)
end)

after_each(function()
  foo.validate:revert()
end)

it("Validate stub foo", function()
  assert.same(foo.validate(), false)
end)
```

In APICast all ngx variables are reset on `ngx_helper.lua` but some
methods like `ngx_variables` or `backend_client` are not reset and the
mock function is in there for all other test.

The test in `spec/policy/ngx_variable_spec.lua` validates that
`ngx_variable.available_context` is working correctly, due some other
test can mock this function the test was inestable over the last days.

This commit creates a way to define a cleanup for mocked functions that
are not part of the ngx class.
@mikz
Copy link
Contributor

mikz commented May 20, 2019

All mocks/stubs should be reverted automatically. If they are applied in some global context (not after/before_each), then they would not be reverted between tests.

I see this change as a bandaid and yet another way to rollback changes. Why we can't just rely on the rollbacking the stubs?

@eloycoto
Copy link
Contributor Author

@mikz I'm ok to apply the :revert() function on the after_each functions, I think that it's a bit clear and It makes things easier to read and debug.

Last week, I had a chat with @davidor and we agreed that we should use the same way that we use for ngx.* methods.

@mikz
Copy link
Contributor

mikz commented May 20, 2019

@eloycoto but that should not be needed at all. There are global after/before hooks that do that for all registered stubs. There should be no reason to manually revert them because it would be really error-prone.

https://github.com/3scale/APIcast/blob/master/spec/luassert_helper.lua (was significantly refactored in #799).

@eloycoto
Copy link
Contributor Author

Will check, thanks!

@eloycoto
Copy link
Contributor Author

Close this one due to is not the correct solution, the correct one is on #1042

@eloycoto eloycoto closed this May 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants