Page 14 of 15

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Wed Apr 30, 2008 6:34 am
by Chris Corbyn
Ok, I'm happy with that load() method.

Now it's just save().

We've got two options:

1) Ask the model to save() some data to a file then do a file_get_contents() and analyze the string
2) Ask the model to save() some data to a file then include() that file and check the array itself

Which do we prefer. I see potential problems with each. Can anyone think what the potential problems with the above two methods are?

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Wed Apr 30, 2008 8:49 am
by matthijs
In both cases it's about making sure the data are saved correctly isn't it?

Can't you re-use the load method for that? Or have both methods (load and save) use a common private method? I mean, both methods need to read/include the file. Only the save() methods must confirm that the same data is in the file as what was supposed to be saved.

I'll have to think about this. Have little experience with file functions, time to catch up I guess.

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Wed Apr 30, 2008 9:05 am
by Chris Corbyn
Ok, more specifically, can you see any potential issues with this test?

Code: Select all

public function testSimpleArrayIsSavedToFile() {
  $this->_createStorageModel('test.php', 'arr')->save(array('zip' => 'button'));
  include 'test.php';
  $this->assertEqual(array('zip' => 'button'), $arr);
}
EDIT | Hint - I'm referring to the case where our test should fail because the implementation is incorrect.

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Wed Apr 30, 2008 9:53 am
by matthijs
Maybe you mean the case in which the array $arr is appended to the file, instead of overwriting the existing data?

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Wed Apr 30, 2008 5:32 pm
by Chris Corbyn
matthijs wrote:Maybe you mean the case in which the array $arr is appended to the file, instead of overwriting the existing data?
Well yes, that would still make the test pass and not really be desirable ;) But what I was getting at was this?

Code: Select all

<?php
 
$array = array(
  "foo" => "bar
  );
 

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 12:46 am
by matthijs
Earlier I mentioned
matthijs wrote:Or a test which tests that when you try to load a non existing file or non existing array it returns false. And maybe a test which asserts that when you load a faulty array it also returns false.
You mean that?

And then testing it with
public function testLoadingNonExistingFileReturnsFalse(){}
public function testLoadingNonExistingArraynameReturnsFalse(){}
public function testLoadingFaulthyArrayreturnsFalse(){}

For example

Code: Select all

 
  public function testLoadingFaulthyArrayreturnsFalse(){
    $this->_createTestFile('test.php', '<?php $data = array("foo" => "bar); ?>');
    $this->assertFalse( $this->_createStorageModel('test.php', 'data')->load() );
  }
 

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 3:05 am
by matthijs
Ok, I don't know how to solve this. I studied every file function there is in the manual, but I'm missing a simple is_parseble() function, to be able to check if an included file is parseble PHP.

As soon as you deal with a file

Code: Select all

 
<?php $data = array("foo" => "bar); ?>
 
 
Trying to include that leads to a parse/syntax error.

Reading the file with something like file_get_contents or one of the other read functions is possible, but then again you have to parse the content to check if it's valid PHP. And I assume we are not going to write our own parser.

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 4:37 am
by arjan.top
there was such a function but it was removed:
http://si2.php.net/manual/en/function.p ... syntax.php

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 5:05 am
by matthijs
Mm, too bad. There are some people in the comments on that page offering home-made solutions. But there must be a simple solution I'm over looking. I mean, including a file and checking if it's parsable, you'd say that's one of the first functions ever made in PHP?

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 6:24 am
by Scrumpy.Gums
Is there some way that SimpleTest can expect an error? I'll have a look at the docs later on when I have time :)

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 6:35 am
by Chris Corbyn
There isn't a way it can expect a syntax error no ;) Syntax errors happen at compile time so the test simply wouldn't run. The solution to this is not to include() the file, even though that is the obvious way to do it. Obviously include()'ing files generally is fine, but to include() a file which you are generating from a test could completely halt a test suite. Generally I'd prefer to avoid anythign which may blatantly cause a compiler error. It's not so bad in Java as it is in PHP since in Java the compiler runs very early, but in PHP it happens as the files are interpreted. If we had a test suite with 300 test cases in it and the 270th test which is run after potentially 60 seconds processing time has code like this in it you'd be very frustrated and potentially left wondering which of the 300 test cases was producing the error (2 years down the line you're asking yourself "tmp/test.php... Hmm... which test would I have put a file like that in"?).

I'm not strongly opposed to the idea of doing it that way because at least it's simple, and an error would be a signal that the test is failing; it's just that given the potential risk I think a string comparison would pay-off better.

The alternative is something like this, but it comes with it's own test-fragility problems too.

Code: Select all

public function testSimpleArrayIsSavedToFile() {
  $this->_createStorageModel('test.php', 'arr')->save(array('zip' => 'button'));
  $this->assertEqual(
    '<?php' . PHP_EOL .
    '$arr = array(' . PHP_EOL .
    "'zip' => 'button'" . PHP_EOL
    ');',
    trim(file_get_contents('test.php'))
  );
}
What's the major issue here?

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 6:45 am
by Scrumpy.Gums
Ah yes. From the SimpleTest docs:
Note that SimpleTest cannot catch compile time PHP errors.
;)

The major issue I guess would be that even if the syntax was valid, it could still fail the test due to formatting.

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 7:20 am
by Chris Corbyn
Correct :) Any better assertion than assertEqual() you can think of?

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 7:45 am
by matthijs
assertIdentical() or even assertWantedPattern() and then use some regex. But those won't prevent the syntax error crash

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 7:54 am
by Chris Corbyn
matthijs wrote:assertIdentical() or even assertWantedPattern() and then use some regex. But those won't prevent the syntax error crash
They'll prevent a syntax error problem. file_get_contents() just reads the file contents... it won't try and parse it like PHP does.

I agree that assertPattern() is what we need. Can't use assertWantedPattern() since it's been @deprecated for ages and has now been deleted from the repository ;) assertPattern() replaced it.

The only annoying thing with assertPattern() for something like this is that it's hard to convey what you're trying to test. It's extra important to use a descriptive method name, keep the test as concise as possible and of course, add a assertion message.

*chuckles* :lol: Who's got their regex head on? Somone wanna take a stab at a nice flexible regex for this?