Assume test assets are precompiled

[jrafanie]

Extracted from #18076

Why? First of all, live compiling of assets is true by default in the
test environment in rails so we need to get back to the defaults.

Sadly, this causes a 2 minute delay running any UI spec that calls
image_path/asset_path directly or indirectly.

Flamegraphs from rbspy show that the 2 minutes delay running a local
ui-classic test that calls image_path/asset_path is in sprocket-rails
in the call to asset_precompiled? which calls precompiled_assets(true)
if you're not caching classes, such as test. It then needs to look
through all asset paths for precompiled assets to build a manifest and
ends up calling stat_tree / stat_directory in all these paths, such
as the node_modules. We can assume the assets are precompiled as we
compile them on demand and can avoid this 2 minute stat'ing of files and
directories. It's unclear if this has any negative consequences.

https://github.com/rails/sprockets-rails/blob/v3.2.1/lib/sprockets/railtie.rb#L39

[miq-bot]

Checked commit jrafanie@f408d98 with ruby 2.3.3, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐

[Fryguy]

cc @martinpovolny @himdel Thoughts?

[jrafanie]

Note, I have had this running against my rails 5.1 tests for ui-classic and manageiq-api and they're green.

[martinpovolny]

Sadly, this causes a 2 minute delay running any UI spec that calls
image_path/asset_path directly or indirectly.

not any, but first

Yes. therefor we disabled it.

First of all, live compiling of assets is true by default in the test environment in rails so we need to get back to the defaults.

Why do we need get back to the defaults?

ping @himdel

[jrafanie]

not any, but first

Very true. Thankfully, it's only the first time that we hits this. It's awful.

Why do we need get back to the defaults?

ping @himdel

The alternative to the default of compile: true is that rails 5.1 (sprockets) yells [1] if it can't find an asset in the pipeline and tries to falls back to looking in the public folder, regardless if it exists in the public folder. This means, we either need to precompile all assets for travis AND local tests... OR we need to "live" compile it via compile: true.

[1] DEPRECATION WARNING: The asset "XXX" is not present in the asset pipeline.Falling back to an asset that may be in the public folder.
This behavior is deprecated and will be removed.
To bypass the asset pipeline and preserve this behavior,
use the `skip_pipeline: true` option.

If there is a problem with the compile: true and this monkey patch in test.rb, we can closet this PR. Without any failures, it feels like this is the simplest solution, especially if our usage of sprockets is becoming less and less.

I feel like sprockets-rails should be changed to not stat all the asset paths on first call to image_path/asset_path in the test environment but I just don't understand the use cases of assets with rails in test to offer a pull request there.

[jrafanie]

@himdel @martinpovolny I replied above but neglected to mention you...

[jrafanie]

Note, if we keep this logic, it will have to also add the spec and api test suites since they would also need to precompile their assets instead of live compile them.

It feels like live compilation is less intrusive.

[himdel]

LGTM,
I'm not noticing any slowdown, either when running individial tests (./spec/views/layouts/listnav/_ems_cloud.html.haml_spec.rb, ManageIQ/manageiq-api#606 (comment), 11s both ways),
or when running the whole suite (7m28s +-1s both ways).

(And, as mentioned in ManageIQ/manageiq-api#606 (comment), most of the remaining code using the asset pipeline will go away when we move patternfly to webpack in ManageIQ/manageiq-ui-classic#5679.)

[himdel]

(Also confirming ui-classic specs still pass with the change 👍.)