Skip outputReady in production builds

[mehulkar]

The outputReady hook shouldn't need to run in production builds:

ember-cli-fastboot-testing/index.js

Lines 49 to 50 in 441d4f9

	// we have to use the outputReady hook to ensure that ember-cli has finished copying the contents to the outputPath directory
	outputReady(result) {

Seems implementing isEnabled() will do this (currently only in the included hook:

ember-cli-fastboot-testing/index.js

Lines 25 to 28 in 441d4f9

	const isEnabled =
	this.app.name === 'dummy' || this.app.env !== 'production';
	if (!isEnabled) return;

---

The specific issue I'm running into is that result.directory is wrong when this hook is called (I'm trying out embroider and not yet sure if it's an issue with that or something else).

Details about the failure

When ember-cli calls the outputReady hook, the directory is correct (with my custom --output-path):
https://github.com/ember-cli/ember-cli/blob/26b2ee9f1b9f3431d7b96a7f850464007281eb35/lib/models/builder.js#L152

[ember-cli:builder] buildResults.directory /var/folders/dw/l307h_b16vzc0wys2f_nbmm00000gn/T/broccoli-121748JRezg9nx2Ji/out-1244-packager_runner_embroider_webpack
[ember-cli:builder] outputPath /dev/my-app/custom-dist-path
[ember-cli:builder] combinedOptions .directory /dev/my-app/custom-dist-path

CALLING outputReady for fb-testing with results.directory /dev/my-app/custom-dist-path

But inside the hook itself, it's wrong, using some default value

[ember-cli-fastboot-testing] process.env.EMBER_CLI_TEST_OUTPUT undefined
[ember-cli-fastboot-testing] result.directory /dev/my-app/dist <----- THIS IS WRONG

My only guess as to what's happening is that the object is getting updated somehow, but I'm not sure.

[ryanto]

Ah bummer, sorry you're running into this!

It sounds like doing the same isEnabled check in outputReady is would solve this problem? If you want to PR that I think we could merge it.

@SergeAstapov any thoughts on this?

[SergeAstapov]

@ryanto @mehulkar I think it should be safe/fine to wrap logic in the outputReady in the condition like we have in the included hook.

Sounds like it's embroider specific as didn't see any issues neither in the CI of this addon, our large app or other adding that use ember-cli-fastboot-testing in CI.

I'll poke around outputReady hook and create PR unless @mehulkar beats me.

[mehulkar]

Yep adding the same check would be ok.

[backspace]

I was having deployment problems with an Embroider application and the linked PR fixed it! 🎉 I think it’s good to merge 🤓