Test if dateString is set and not empty in createDateTimeFromStr

[nicolasrabier]

Some OFX files don't have all the date tags. So when the value of the date tag that doesn't exist and it is passed as a parameter to $dateString into the function "createDateTimeFromStr", this function returns

=> Failed to initialize DateTime for string:

To bring more flexibility, I suggest testing first if dateString is set and is not empty before proceeding to the conversion.

[nicolasrabier]

Hi @asgrim, Just wondering if you expect me to do anything on this issue. Thanks

[asgrim]

@nicolasrabier yes, please this PR needs tests (as does the other in #51 please) - thanks!

[nicolasrabier]

@asgrim I'm more a Java developer. Not certain how to do it in PHP and this environment. Do you have any documentation? Or could you point out where to add the test? Thanks

[asgrim]

@nicolasrabier of course - the test suite isn't great but it's something I definitely want to improve, but I'll comment here and on the other issue about the best way of testing :)

If you can't get to it, don't worry, I'm sure me or someone else can add tests eventually :)

https://github.com/asgrim/ofxparser/blob/93ddca0acc2bf0a688f1f98a5f122048e6cbd832/tests/OfxParser/OfxTest.php#L65-L90

This is where we'll want to add a test case for this one (this should be broken up into a @dataProvider but don't worry too much if not).

It might also make sense to add an integration test with blank date values to make sure nothing crashes in this test too:

https://github.com/asgrim/ofxparser/blob/93ddca0acc2bf0a688f1f98a5f122048e6cbd832/tests/OfxParser/ParserTest.php#L159-L170

[nicolasrabier]

@asgrim Thanks for the clarification. I'll have a look and start implementing the tests.

[nicolasrabier]

Hi @asgrim As you can see, I did commit the tests for both fixes (#51 & #52). Let me know if you want me to change anything.

I struggled with setting up the local test environment. Not sure if I did it in the right way but I had to create a autoload.php file with phpab and changed the extended class name of the test classes because the PHPUnit_Framework_TestCase class wasn't found; I locally replaced \PHPUnit_Framework_TestCase by \PHPUnit\Framework\TestCase. Maybe it was due to my version of phpunit (mine: PHPUnit 7.5.2). Which one does the project use?

[asgrim]

Typically in a modern PHP project you'd install the dependencies (and thus things like phpunit) with Composer, like:

composer install

This would install the right version of phpunit etc, then you'd run vendor/bin/phpunit to run the whole test suite, or vendor/bin/phpunit tests/OfxParser/OfxTest.php to run a single test file for example (there's more ways, but this is a couple of basic examples!). The namespace difference is indeed because we're using an ancient phpunit; there's a lot I need to update in this library... but $time is null at the moment :wink:

That said, the tests are added and passing, so this is all good, thank you for fixing up :)

[nicolasrabier]

Thanks @asgrim for the explanation. I will try composer install out of curiosity. Good luck with the project.