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

PHPUnit tests should not have database side effects #458

Closed
Trott opened this issue Feb 14, 2014 · 9 comments
Closed

PHPUnit tests should not have database side effects #458

Trott opened this issue Feb 14, 2014 · 9 comments
Milestone

Comments

@Trott
Copy link
Contributor

Trott commented Feb 14, 2014

When fixed, be sure to take second phing run out of .travis.yml

@saschaben saschaben added this to the v2.4.1 milestone Feb 18, 2014
@saschaben saschaben modified the milestones: v2.4.2, v2.4.1 Apr 4, 2014
@stopfstedt stopfstedt modified the milestone: v2.4.2 Apr 24, 2014
@stopfstedt
Copy link
Member

I think we left it at labeling end-to-end tests appropriately and gave up on the idea of mocking up the database for those. see #535

@stopfstedt
Copy link
Member

@Trott can this be closed?

@Trott
Copy link
Contributor Author

Trott commented Apr 30, 2014

Looks like @jrjohnson wants to deal with this a different way. See #577. Basically, refresh the database between every test. Works for me. Once that's merged (which I'm about to do), I'll close this ticket too.

@jrjohnson
Copy link
Member

Though I think we need to talk about testing artifacts more broadly I had no other solution for the problem of unique constraints in a test (in this case course name to program year). The only way to run another behat test after the first one was to reset the DB between. I think we can go ahead and close this now, since we are going to have artifacts from most tests anyway.

@Trott
Copy link
Contributor Author

Trott commented Apr 30, 2014

Oh, I got it wrong. #577 is all about Behat tests, which can have side effects. It's the PHPUnit tests that shouldn't. You might be right, we may have nuked it.

@Trott Trott reopened this Apr 30, 2014
@Trott
Copy link
Contributor Author

Trott commented Apr 30, 2014

This is about PHPUnit tests. I'll check and make sure that there aren't still side effects, but if there are, we should leave this open until we've removed them. @stopfstedt may be right though. They may be gone. Sorry about the confusion.

@jrjohnson
Copy link
Member

Tests have side effects and until we start tearing out the model layer and putting in things like in-memory temp databases, mock objects, and data fixtures we are going to be stuck with some database side effects for any effective test run. I don't see any reason that phpunit would be treated differently from behat.

@Trott
Copy link
Contributor Author

Trott commented Apr 30, 2014

Unit tests should not have side effects. Integration tests (or whatever you want to call Behat tests) would be expected to have side effects. To the extent that we are using PHPUnit to run tests that are not, in fact, unit tests, then yeah, there's going to be side effects. And that's fine, we mark them as end-to-end tests or whatever and move on. But for genuine unit tests (or things that should be genuine unit tests), this issue should be left open until we start using test doubles or whatever else to isolate the unit tests.

At least, that's how I see it.

I'm open to charm and persuasion.

@Trott
Copy link
Contributor Author

Trott commented Apr 30, 2014

OK, so the way it looks: There are definitely still side effects in the PHPUnit tests. But the CodeIgniter/PHPUnit combo does not really make for easy test doubles for things like database backends. Closing as a WONTFIX or WILL-GET-TO-WHEN-WE-MIGRATE-TO-A-NEW-FRAMEWORK-AND/OR-PLATFORM.

TL;DR: @jrjohnson and @stopfstedt r00l, @Trott dr00ls

@Trott Trott closed this as completed Apr 30, 2014
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

No branches or pull requests

4 participants