Unit tests include hardcoded paths

[mfendeksilverstripe]

I found this lovely piece of code in the AnnotatorPageTest.php. We moved Page and PageController to a different location (under Pages folder) in our project. Since these paths are hard coded unit tests are now failing. At least the code comments were honest about the situation.

// Why is this required?
// @todo get it to work properly
require_once(BASE_PATH . '/mysite/code/Page.php');
require_once(BASE_PATH . '/mysite/code/PageController.php');

I recommend avoiding using hard coded paths for includes if you're including stuff from project folder.

[robbieaverill]

We've got this logged as #93, but this has a bit more detail so I'll close that in favour of this one.

We shouldn't need these require statements at all - do you feel like making a PR to remove them @mfendeksilverstripe?

[Firesphere]

I'd love to get rid of this weird requirement! I've not found a reason for it, but if you have anything, please do make a PR @mfendeksilverstripe :D

[Firesphere]

Sorry, wrong button!

[Firesphere]

I have not yet gotten to the bottom as to why these use statements are needed in the tests. Any help is welcome

[Firesphere]

It still requires the require, I would like to get it out for 3.x stable, but I can't find where it is, for the love of all that is holy and unholy

[Firesphere]

Refering to #93 and closing this one

[Firesphere]

Reopening because I did not think clearly.

[Firesphere]

Although it's weird/stupid this is somehow needed, how come your tests are failing @mfendeksilverstripe ? You shouldn't run vendor tests in your own code.

I agree it shouldn't be there, but it should also not affect your code?

[mfendeksilverstripe]

@Firesphere I actually don't remember what exactly the issue was as we don't use this module anymore. I will have a look at it on Friday (Hackday).

[mfendeksilverstripe]

@Firesphere I included the module and ran the project tests. No issues were present this time.

[robbieaverill]

Ideally these classes would be loaded by composer. If this module doesn't use the SS class manifest and there's no autoloader definition for these classes then it might not work, which could be a reason why we'd need to keep this. If that's the case then it'll become a "meh, oh well" for now - we could look at making a PR to add a PSR-0 autoloading rule for them, but (in SS4) they're added as project files by recipe-cms so not sure whether the current recipe plugin would support adding those extra composer.json configs - I assume it probably won't

[Firesphere]

Still an open issue. Slightly resolved in #107, which would at least check the classes exist. Proper solution implementation is only relevant for people actively working on this module and not end users.

Won't fix in the short term, but is on the list.

[robbieaverill]

Ah, I think it's because those stubs are suffixed with "Test" so PHPunit is trying to run them. I'll rename them

[robbieaverill]

Fixed in #116

[Firesphere]

Merged in #116