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

Type-check stubs #58

Merged
merged 2 commits into from
Oct 19, 2020
Merged

Type-check stubs #58

merged 2 commits into from
Oct 19, 2020

Conversation

randycoulman
Copy link
Contributor

Part of #56

Currently, only mocks created with expect are type-checked by Hammox.

This provides the same protection for stubs created with stub.

There is also stub_with, but I'm not sure how to add protection for that one; suggestions welcome!

I wasn't sure how best to test this change. For now, I modified assert_pass and assert_fail to use stub instead of expect; there are a few other tests that continue to use expect, so both code paths are being tested. I'm happy to change this up however you'd like.

I extracted a helper named wrap that is now used in both expect and stub; I'd rather use the name protected for this, but that name's already taken. Again, I'm happy to change this however you'd like.

@msz
Copy link
Owner

msz commented Oct 11, 2020

Thanks! I think the code looks great.

For testing, I'd go a simpler way — I'd just make a test where we stub with a wrong function and try to call the stub, then assert that there was a Hammox exception thrown. This should ensure that stubs are being protected.

@randycoulman
Copy link
Contributor Author

@msz Great, thanks! I'll take care of that tomorrow.

If you have a minute to drop a hacktoberfest-accepted label on this PR, I'd appreciate it. Thanks!

Randy Coulman added 2 commits October 12, 2020 08:00
Closes msz#56

Currently, only mocks created with `expect` are type-checked by Hammox.

This provides the same protection for stubs created with `stub`.
Revert earlier change to `assert_pass`/`assert_fail`.
@randycoulman
Copy link
Contributor Author

@msz I've updated the tests as you suggested; I reverted my earlier change and added explicit tests of expect/4 and stub/3 near the bottom of the test file.

Copy link
Owner

@msz msz left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@msz msz merged commit 2e6636d into msz:master Oct 19, 2020
@randycoulman randycoulman deleted the feat/protect-stubs branch October 23, 2020 22:14
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