Closed jonleighton closed 7 years ago
Excellent. Thanks for spotting this and supplying this change. I'll review this one but I'm still on the road for a couple more weeks. All makes sense, will merge once I've taken a proper look in a few weeks.
@jonleighton thanks again for this. Sorry it's taken me a little while to merge - been busy getting back in to the groove after travelling.
No worries :)
In Rails' default test environment configuration, a presenter statically specified in app/presenters/ would fail to be loaded by PushType::Presentable. This is because the Object.const_defined? check would return false, and so PushType would dynamically define its own version of the presenter class, thus ignoring the presenter class definition on disk.
This stems from the test environment configuration which has cache_classes set to true and eager_load set to false.
With this configuration, Rails won't try to load the file from disk when Object.const_defined? is called. (I'm not sure the exact reason for this.)
In development mode, we have cache_classes and eager_load both set to false. This works fine.
In production mode, we have eager_load set to true, which obviously means that any presenters defined in app/presenters will be loaded up-front when the application starts. So this also works fine.
Therefore, the problem is only noticeable in the test environment.
Note that I'm generating a PagePresenter in the dummy app, there are two reasons for this:
It causes an app/presenters/ directory to get generated, which is needed to put the presenter in which is used in the test I've added (the directory must exist at application boot time, so that Rails adds it to the autoload_paths)
It adds surface area and diversity to the test suite by exercising the presenter generator too