Do not use angular debug mode unless in development

[ppisljar]

Disables angular debug info if dev mode is not enabled to increase performance of release builds

fixes #7125

[ppisljar]

i see test server sets environment name to development, so i decided to only disable debug info if environment name IS SET to production. should i rather disable debug info if environment name IS NOT SET to development ?

[epixa]

We have a dev_mode plugin in core_plugins that only runs if the app is in dev mode - can you tie into that instead of doing a dev check in ui/chrome?

We should preserve the existing logic for checking whether it's considered "development" rather than introducing a new concept (i.e. not-production). Using existing flags helps ensure this: https://github.com/elastic/kibana/blob/master/src/core_plugins/dev_mode/index.js#L2

[ppisljar]

i cant plug it in dev mode plugin, because 1) by default debug info is enabled ... we need to disable it. cant do it in dev mode plugin as it only runs in dev mode 2) actually debug info shouldn't only be enabled in dev mode but also to run test (which don't run in dev mode). that's why i used environment name (which seems to be an existing setting)

[ppisljar]

--dev does not come thru in tests ... i had to add it to the default options .... which i dont like ... but its working.

[ppisljar]

jenkins, test this

[epixa]

Does this still allow us to run individual test suites? At a glance, it seems like this change would make it so you can only run tests successfully if they're all done together.

[ppisljar]

no it still works as before

[epixa]

This change makes it so we can't run the selenium tests against Kibana builds unless they're running in dev mode, which is definitely not ideal. This seems like a problem with our selenium tests more than anything else, but I'd rather not put the project in a state of limbo my merging this while that is the case. Can you look into what it would take to get the selenium tests to work with the angular debug mode disabled?

To reproduce what I'm talking about, just outright disable the angular debug thing rather than wrapping it in a conditional, and then run npm run test:ui.

[epixa]

Other than that, this seems like a huge win!

[ppisljar]

i see ... thats weird, selenium tests should not depend on debugInfo being enabled.

10:52:45.193: self.checkForKibanaApp()
10:52:46.544: current application: undefined
10:52:46.546: kibanaLoaded = false
10:52:46.547: Kibana is not loaded, retrying

here we try to use .scope() which will return undefined if debug info is disabled. I'll check with @LeeDr what we could do about this.

but at the moment our selenium tests do seem to be running in dev mode ? it seems kibana server is started normally but when i check for internals.devFlag its set to true (so debugInfo is not disabled in current PR when selenium tests are running)

[LeeDr]

The selenium tests don't normally run with Kibana running in dev mode. I don't think they can because of the basepath inserted in the URL. We might be able to work around that in the selenium tests if we needed to.

I don't know anything about the angular debug info. If we had to change the method in the selenium tests that checks if Kibana is loaded, we certainly could. That whole set of methods for loading pages is a bit of a mess.

[epixa]

You can run Kibana in dev mode and disable the basepath proxy, so I assume that's what's going on.

[ppisljar]

this could be merged in, but as @epixa mentioned it will prevent selenium tests from running unless they are running in dev mode ( which is currently the case ).

in the future we need to update selenium tests to not depend on angular debug info. if running selenium tests in non dev mode is a requirement then this PR needs to wait until selenium tests are updated.

[epixa]

Yeah, running selenium tests outside of dev mode is pretty important, so they should be updated to support that.

[LeeDr]

I'm still a bit confused. I'm not sure if the current Selenium tests on Jenkins run in dev mode, but I often run the tests against a production install or release candidate on my laptop. In fact, I've made other changes just to support that better.

But if this change potentially breaks the getApp() method that uses .source() to check for Kibana, we can certainly change that (if that's the only thing that breaks).
https://github.com/elastic/kibana/blob/master/test/support/page_objects/common.js#L201

[epixa]

@LeeDr You hit the nail on the head, it seems. That function is relying on an angular debug thing that this PR is trying to remove unless you're running in dev mode because it has a negative impact on performance, so if this PR were to be merged, the tests would only be runnable in dev mode, which would be a problem.

We just need to update the page object(s) to not rely on that debug behavior.

[ppisljar]

waiting for your review :)
dependency on angular debug mode in sellenium tests is removed

[LeeDr]

@epixa I tested the changes @ppisljar made to test/support/page_objects/common.js in this PR where the dependency on angular debug mode in Selenium tests is removed.

[epixa]

jenkins, test this

[epixa]

^ I just want to see an updated selenium build with this change.

[epixa]

@ppisljar Can you rebase this to get the CI tests to run properly?

[epixa]

Successful selenium build: https://kibana-ci.elastic.co/view/All/job/elastic+kibana+pull-request+multijob-selenium/682/console

LGTM