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

[spec] Replace custom assert[Not]AlmostEquals with[_not].near #3885

merged 1 commit into from Apr 15, 2018


None yet
2 participants

edorfaus commented Apr 15, 2018

Also, change "make static-check" to also check the tests.

I seem to be getting further and further away from what I originally intended to do, I'm not even sure I'm still yak shaving, but hey, as long as it improves the codebase in some way or other... (I expect you'll reject a PR if it doesn't, or at least ask for improvements if the idea is good but the implementation bad.)

This one is pretty much a result of me taking a look at busted, noticing custom assertions was possible, finding these functions, and figuring that making them follow the busted way of doing it would be cleaner... and while doing that, and looking at how busted/luassert deal with some details, noticing that there's already a standard assertion doing the same thing. Well, hey, even cleaner! :)

[spec] Replace custom assert[Not]AlmostEquals with[_not].near
Also, change "make static-check" to also check the tests.

This comment has been minimized.


Frenzie commented Apr 15, 2018

As an aside, since many tests were written for busted 1.x it's possible that it simply wasn't around.

I'm all for moving from custom stuff to upstream stuff. See for instance 53c0756#diff-f92786a1f129cd64facf7434893912ee where I moved from our own verbose print to upstream gtest. (Wasn't around in 1.x.)

@Frenzie Frenzie merged commit f12f317 into koreader:master Apr 15, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!

@edorfaus edorfaus deleted the edorfaus:almost-equals=near branch Apr 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment