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

Issues with the Unit Test Suite #43

Closed
wtfzdotnet opened this issue Jul 31, 2014 · 6 comments
Closed

Issues with the Unit Test Suite #43

wtfzdotnet opened this issue Jul 31, 2014 · 6 comments
Assignees
Milestone

Comments

@wtfzdotnet
Copy link
Member

Continuing from #42 on wards, @mihaeu maybe you can create a generic outline?

@mihaeu
Copy link
Contributor

mihaeu commented Jul 31, 2014

Well the thing I've noticed when adding 0c72e82 is that, with the current test setup I couldn't write actual assertions and that for some of the tests there are no assertions. The coverage of the tests is good, but they don't actually confirm that everything works as it is supposed to.

This becomes apparent when running PHPUnit with the --report-useless-tests flag:

useless-tests

So basically we would need to check that the core features (i.e. getting data from the repositories) actually works.

To keep the tests fast there are already offline .json files which serve as mocked API responses, but in the tests I was looking at they were not used.

Did I get everything right?

@wtfzdotnet
Copy link
Member Author

@mihaeu any chance we can collaborate on this soon? Feel like tackling this issue :-), got some spare time at hands here and there.

@mihaeu
Copy link
Contributor

mihaeu commented Sep 10, 2014

Hey there, sry I recently married and am in-between jobs so spare time
is scarce here :)

I would still like to get it done, just can't make any promises about
when that's going to be :(

On 09/10/2014 12:45 PM, Michael Roterman wrote:

@mihaeu https://github.com/mihaeu any chance we can collaborate on
this soon? Feel like tackling this issue :-), got some spare time at
hands here and there.


Reply to this email directly or view it on GitHub
#43 (comment).

@wtfzdotnet
Copy link
Member Author

Congratulations first off all ;)! Been on your honeymoon as well?

Alright I'll take a dive into it myself and see how far I'll get, I'll keep you updated.

@wtfzdotnet
Copy link
Member Author

@mihaeu I've been working hard on improving on this in the 2.0-WIP branch, by the end of today the risky tests should equal 0. I used to assume that PHPUnit would actually assert on the fact a get requests was made ( even how naive that was ).

I'd invite you to take a look at a bunch of these changes made in 870ed68, should be way improved.

@wtfzdotnet
Copy link
Member Author

Since all risky tests are gone now, I'm closing this issue and continue onwards to imrpove the unit test suite quality in #51

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

No branches or pull requests

2 participants