Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Test cases for Alternative GridFS collection names issue #766. #777

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

3d0c commented Jan 7, 2013

Test cases for issue #766

@jails jails commented on an outdated diff Jan 7, 2013

tests/cases/data/source/MongoDbTest.php
+
+ /**
+ * This test fails in current conditions.
+ */
+ public function testCreateGridfsWrongPrefix() {
+ $source = 'somename.files';
+
+ $model = $this->_model;
+ $model::config(array('meta' => array('source' => $source, 'connection' => 'default', 'locked' => false)));
+
+ $data = array('file' => 'xxx', 'filename' => 'xxx');
+
+ $model::create()->save($data);
+ $result = $this->db->connection->gridFSinstance;
+
+ // Expected this will fail.
@jails

jails Jan 7, 2013

Contributor

If you expect an exception you need to use something like: $this->expectException('/The error message regexp/');

@jails jails commented on an outdated diff Jan 7, 2013

tests/cases/data/source/MongoDbTest.php
@@ -2,7 +2,7 @@
/**
* Lithium: the most rad php framework
*
- * @copyright Copyright 2013, Union of RAD (http://union-of-rad.org)
+ * @copyright Copyright 2012, Union of RAD (http://union-of-rad.org)
@jails

jails Jan 7, 2013

Contributor

Any chance having this PR rebased on HEAD dev ?

@jails jails and 1 other commented on an outdated diff Jan 7, 2013

tests/mocks/data/source/MockMongoConnection.php
@@ -57,6 +59,11 @@ public function find($conditions, $fields) {
public function listCollections() {
return $this->_record(__FUNCTION__);
}
+
+ public function getGridFS() {
@jails

jails Jan 7, 2013

Contributor

Here getGridFS() should take the same argument you have provided in your PR (e.g. getGridFS($prefix = 'fs')) imo.

@3d0c

3d0c Jan 8, 2013

Contributor

I'm afraid — no, because this method emulates current lithium behaviour — this is a main problem, i wish to solve.
Take a look into lithium\data\source\MongoDb line 412

$grid = $this->connection->getGridFS();

This call never returns something other than default gridfs collection.

@jails

jails Jan 8, 2013

Contributor

What I meant here is you need to build your test upon your PR (which solve the problem of always returning the default gridfs collection). In #766 you changed:

$grid = $this->connection->getGridFS();

to:

$grid = $this->connection->getGridFS($this->_config['gridPrefix']);

So your mock should be representative of the class you are mocking. I don't think you'll be able to build valid tests otherwise. This way you only need to test if passed parameter is correct over different CRUD operations.

@jails jails commented on the diff Jan 7, 2013

tests/cases/data/source/MongoDbTest.php
+ public function testCreateGridfsAlternativeName() {
+ $source = 'somename.files';
+
+ $this->_testConfig['gridPrefix'] = 'somename';
+ $this->setUp();
+
+ $model = $this->_model;
+ $model::config(array('meta' => array('source' => $source, 'connection' => 'default', 'locked' => false)));
+
+ $data = array('file' => 'xxx', 'filename' => 'xxx');
+
+ $model::create()->save($data);
+ $result = $this->db->connection->gridFSinstance;
+
+ $this->assertTrue(is_object($result));
+ $this->assertEqual($source, $result->filesName);
@jails

jails Jan 7, 2013

Contributor

Is this test really pass ?

@jails jails commented on an outdated diff Jan 7, 2013

tests/mocks/data/source/MockMongoGridFS.php
@@ -0,0 +1,50 @@
+<?php
+/**
+ * Lithium: the most rad php framework
+ *
+ * @copyright Copyright 2012, Union of RAD (http://union-of-rad.org)
@jails

jails Jan 7, 2013

Contributor

Happy New Year 2013 ;-)

Owner

nateabele commented Jan 7, 2013

@3d0c Sweet! Thanks for the comprehensive patch. @jails Thanks for going through it and making proper notations.

Owner

nateabele commented Jan 8, 2013

Looks like the build failed for internal Travis reasons. Anybody know how to trigger it to run again on a pull request?

Member

blainesch commented Jan 8, 2013

Not easily on non-master branch. You can change something slightly in the last commit and force push... or do a blank commit git commit --allow-empty push it then remove it and force push? This first push will trigger a travis fail since it won't be able to find it, the second one will pass.

Sounds messy.

Owner

nateabele commented Jan 8, 2013

What's weird is, he just added another commit, which should have triggered another build, but I'm not seeing any.

Contributor

jails commented Jan 8, 2013

Yeah sometimes Travis plays his Joker...

Member

blainesch commented Jan 8, 2013

Travis may be lagging. Amazing free service needs more funding lol

Could you run the tests locally?

Contributor

jails commented Jan 8, 2013

Anyway this test can't pass w/o #766.

@jails jails closed this Jan 9, 2013

Contributor

jails commented Jan 9, 2013

merged in #778

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