Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
<?php
class User extends Eloquent {
public function getOldest()
{
return $this->orderBy('age', 'desc')->first();
}
}
@JeffreyWay

This comment has been minimized.

Copy link
Owner Author

@JeffreyWay JeffreyWay commented May 29, 2013

Survey. How would you test the getOldest method?

  1. Don't bother testing it at all.
  2. Write an integration test. Insert a couple test rows into the DB, call the method, and verify that the correct row is returned.
  3. Partially mock the User class, and verify that the orderBy method is called...
  4. Something else??
@MaksJS

This comment has been minimized.

Copy link

@MaksJS MaksJS commented May 29, 2013

2 for me

@kakysha

This comment has been minimized.

Copy link

@kakysha kakysha commented May 29, 2013

2

@sorora

This comment has been minimized.

Copy link

@sorora sorora commented May 29, 2013

4 Refer to "Laravel Testing Decoded" multiple times.

If I've learnt from your book correctly, I believe the "orderBy" class is handled by Eloquent, so that should be returning correct information if it's own tests are working OK.

A quick mock of some rows return (e.g. 10, 20, 40) and ensure 40 is returned otherwise?

@rtablada

This comment has been minimized.

Copy link

@rtablada rtablada commented May 29, 2013

In my current code I don't test.

I would probably test it by creating a user_test table with the same schema temporarily adding a `$table = 'user_test' and then maybe do #2.

Ideally, I would like to mock things out and do it that way... I guess that's what I'm reading your book.

@JeffreyWay

This comment has been minimized.

Copy link
Owner Author

@JeffreyWay JeffreyWay commented May 29, 2013

@sorora - not necessarily for methods like this one...

@bilalq

This comment has been minimized.

Copy link

@bilalq bilalq commented May 29, 2013

2 with a tear down at the end makes the most sense.

@Bronx205

This comment has been minimized.

Copy link

@Bronx205 Bronx205 commented May 29, 2013

3

@desugaring

This comment has been minimized.

Copy link

@desugaring desugaring commented May 29, 2013

2

@wouterj

This comment has been minimized.

Copy link

@wouterj wouterj commented May 29, 2013

3 if this is the only method you want to test, 2 otherwise.

@Mauricevb

This comment has been minimized.

Copy link

@Mauricevb Mauricevb commented May 29, 2013

2

@JeffreyWay

This comment has been minimized.

Copy link
Owner Author

@JeffreyWay JeffreyWay commented May 29, 2013

So here's my issue with #3. Mocking is good, but for short query methods like this, you'd essentially be writing a test that does nothing other than verify that a method, orderBy, is triggered. That's not overly useful.

@sorora

This comment has been minimized.

Copy link

@sorora sorora commented May 29, 2013

@JeffreyWay - it is probably lucky I haven't got onto the Models section of the book then, still much to learn!

@worenga

This comment has been minimized.

Copy link

@worenga worenga commented May 29, 2013

2

@agileurbanite

This comment has been minimized.

Copy link

@agileurbanite agileurbanite commented May 29, 2013

3

@fideloper

This comment has been minimized.

Copy link

@fideloper fideloper commented May 29, 2013

2

@JeffreyWay - I often find I'm doing that (just testing that a method was fired) when I mock tests, especially ones around data. Sorta frustrating for me.

In this situation (and I'm really just thinking this through now as I type)

We know that:

  1. Eloquent\Model is fully tested
  2. Database\Query (and related classes) are fully tested.

Since we know the components ("units") of everything we're using will function (they work correctly as individual units), and we're just USING those components to create a functionality, all that's really left is an integration test to test that the "units", in our implementation, do make up a functional "whole".

So, I think an integration test is what's needed there.

That being said, I still hate using databases in a test. Takes away speed, creates issues of data being present when someone clones and tests, other minor things...

@JeffreyWay

This comment has been minimized.

Copy link
Owner Author

@JeffreyWay JeffreyWay commented May 29, 2013

fideloper - Yeah. Made the survey, because this question comes up all the time.

@nicholasleblanc

This comment has been minimized.

Copy link

@nicholasleblanc nicholasleblanc commented May 29, 2013

2

@Menelion

This comment has been minimized.

Copy link

@Menelion Menelion commented May 29, 2013

2

@DavertMik

This comment has been minimized.

Copy link

@DavertMik DavertMik commented May 29, 2013

@fideloper

That being said, I still hate using databases in a test. Takes away speed, creates issues of data being present when someone clones and tests, other minor things...

It depends on ORM design. In ActiveRecord of Rails or in Doctrine tests run extremely fast because of nested transactions that are rolled back in the end of test. Basically I don't see a problem with integration tests. In my Rails development I use them very often and I'm quite happy to know that I'm testing real things and not something mocked.

I looked into Eloquent and it does not have support for nested transactions :( Perhaps it should, as it's necessary for testing, I think.


As for question, it depends. My answer: use what is cheapest and what provides a better coverage.
2. If I have fixtures on which to test

  1. If I don't have them
  2. If this method is very important to me, but still I don't have fixtures :)
@joezimjs

This comment has been minimized.

Copy link

@joezimjs joezimjs commented May 29, 2013

The integration test should be for Eloquent, but since you're using it more specifically, I'd still do an integration test on this. But, I'd also do a mock for the unit tests. The mock would verify that orderBy is called and give back a mock object. Then you'd also verify that this method is returning the first item in that set.

@jabranr

This comment has been minimized.

Copy link

@jabranr jabranr commented May 29, 2013

2

@ImadBoumzaoued

This comment has been minimized.

Copy link

@ImadBoumzaoued ImadBoumzaoued commented May 29, 2013

2

@JeffreyWay

This comment has been minimized.

Copy link
Owner Author

@JeffreyWay JeffreyWay commented May 29, 2013

So here's how you'd basically do it (for #2).

public function testGetsOldestUser()
{
    Factory::create('User', ['age' => 20]);
    Factory::create('User', ['age' => 30]);

    $oldest = (new User)->getOldest();

    $this->assertEquals(30, $oldest->age);
}
@alexrussell

This comment has been minimized.

Copy link

@alexrussell alexrussell commented May 29, 2013

In general I'd say #1 because this will work (assuming Eloquent works, which we have to) and does not require a test.

I really don't like the idea of 3 (and I've seen you do this quite a bit in some of your testing screencasts recently*) because you're not really testing anything, and you're making the test rely on an implementation detail. The test should test that the method does the right thing, not that it calls a given Eloquent method. But the reasons for not liking this are twofold:

  1. As I mentioned you're not really testing anything. In this case it's not too bad, but someone with a lower knowldge about unit testing might think this type of testing is acceptable for all tests. Give you a false sense of security about the outcome of the testing.
  2. Also as I mentioned, you're relying on implementation details of the code when writing the test. What if (god forbid) this code was changed to use the bog standard DB class - the test would now fail even though the code would stil be correct. Similarly, if the ORM was replaced with a different one that didn't use $this->orderBy(), all your tests fail - yes, great reminder to change the test code, but now you have to update two lots of code.

Finally, the most real proper answer here is that if you want to test (get full coverage or whatever) the integration test makes the most sense. However, from the comments above it sounds like Laravel do not have a test-database-with-fixtures feature out of the box. That's surprising and a bit of a shame.

*I understand in your tutorials you've been doing a very quick intro to testing so that's fine (although if I were you I'd note during the screencast that normally you'd need to test more or whatever).

@alexrussell

This comment has been minimized.

Copy link

@alexrussell alexrussell commented May 29, 2013

@JeffreyWay A-ha I think your 'answer' to #2 answered my question about fixtures (at least to an extent).

@JeffreyWay

This comment has been minimized.

Copy link
Owner Author

@JeffreyWay JeffreyWay commented May 29, 2013

Hey, Alex -

I'd always use mocks if the method did more than simply call a method. This is a different case though. Chris outlined my views well.

@DavertMik

This comment has been minimized.

Copy link

@DavertMik DavertMik commented May 30, 2013

@JeffreyWay
Mocks is a must when you deal with asynchronous storage. Like RabbitMQ, or web services, for instance. It's really hard to check data there.

If connection happens synchronously, there is no actual reason to separate tests from data. Especially when database is properly cleaned and fixtures are used.

@ukoasis

This comment has been minimized.

Copy link

@ukoasis ukoasis commented May 30, 2013

2

@mdespuits

This comment has been minimized.

Copy link

@mdespuits mdespuits commented May 30, 2013

2

When dealing with databases and queries, ALWAYS use integration. Yes, you could use a mock expectation that the method was called, and yes, you can have faith that Eloquent will query properly, but with databases, unless you are dealing with real data, you cannot always be absolutely sure that you did it right. What if you typo-d the method or the parameters in the production and test code? Your tests would pass, but you would have a nasty 🐛.

Mocks are much more useful for asserting that interfaces around external APIs are called properly from your own abstraction, not for testing that the ORM runs the right query and the database returns the right values.

@vladan-zoricic

This comment has been minimized.

Copy link

@vladan-zoricic vladan-zoricic commented May 30, 2013

Some combination of 2 and 3.

@JulienItard

This comment has been minimized.

Copy link

@JulienItard JulienItard commented May 30, 2013

2

@jlambe

This comment has been minimized.

Copy link

@jlambe jlambe commented May 30, 2013

2 because we're dealing with datas from the database. Otherwise 3 if you intend to test a method that format a given data.

@lnguyen0901

This comment has been minimized.

Copy link

@lnguyen0901 lnguyen0901 commented May 30, 2013

2 and 3

@hasokeric

This comment has been minimized.

Copy link

@hasokeric hasokeric commented May 31, 2013

2

@torkiljohnsen

This comment has been minimized.

Copy link

@torkiljohnsen torkiljohnsen commented Apr 2, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.