Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ftr] support --kibana-install-dir flag #44552

Merged
merged 4 commits into from
Sep 5, 2019
Merged

[ftr] support --kibana-install-dir flag #44552

merged 4 commits into from
Sep 5, 2019
+45 −17

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Aug 30, 2019

In certain edge cases we need to access the Kibana installation from the functional test runner via the file system, which requires that we know where it is installed. This should be a last resort option since it means the tests can not be executed against remote instances, but in a pinch this will do the trick.

@spalger spalger added the Team:Operations Team label for Operations Team label Aug 30, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations

@mw-ding mw-ding mentioned this pull request Aug 30, 2019
Merged
7 tasks
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@mw-ding
Copy link
Contributor

mw-ding commented Sep 3, 2019

one issue with this PR is when the --kibana-install-dir param is provided, the local functional test run will use xxx/bin/kibana xxx to spin off kibana instead of node scripts/kibana.js xxx. This will result in "cannot find /bin/kibana xxx` errors in local dev mode.

I thinking of if the functional test config service can provide a param for returning the path.data config value on the fly?

@spalger
Copy link
Contributor Author

spalger commented Sep 3, 2019

The thing is that we don't have access to the actual path.data config value, it could be set a number of different ways and would cause things to get really coupled together if we exposed that to the functional tests. The idea with exposing --kibana-install-dir is that we can do something like:

import { REPO_ROOT } from '@kbn/dev-utils'

const repoDir = resolve(config.get('kbnTestServer.installDir') || REPO_ROOT, 'data/repo/xxx');

When people are running the tests locally they almost always run them against source, so we'll almost always be using REPO_ROOT here, and if they're using a kibana install then they either need to disable this test or pass --kibana-install-dir to make sure that these tests can work.

@spalger spalger marked this pull request as ready for review September 5, 2019 18:26
@spalger spalger added release_note:skip Skip the PR/issue when compiling release notes v7.4.0 v7.5.0 v8.0.0 labels Sep 5, 2019
mw-ding
mw-ding approved these changes Sep 5, 2019
View reviewed changes
Copy link
Contributor

@mw-ding mw-ding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This is super helpful!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger spalger merged commit 16eb033 into elastic:master Sep 5, 2019
This was referenced Sep 5, 2019
Merged
Merged
spalger pushed a commit to spalger/kibana that referenced this pull request Sep 5, 2019
[ftr] support --kibana-install-dir flag (elastic#44552)
fe85b0b 
* [ftr] support --kibana-install-dir flag

* improve spacing/style of success message

* Remove unused var
spalger pushed a commit to spalger/kibana that referenced this pull request Sep 5, 2019
[ftr] support --kibana-install-dir flag (elastic#44552)
5fa741b 
* [ftr] support --kibana-install-dir flag

* improve spacing/style of success message

* Remove unused var
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 6, 2019
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into fix/autocompl…
9e128a9 
…ete-for-distance_feature

* 'master' of github.com:elastic/kibana: (89 commits)
  Replace TSVB timeseries charts with elastic-charts (elastic#33558)
  [TSVB][Top N aggregation] Unable to deal with negative values (elastic#43581)
  [alerting] Adds Action Type configuration support and whitelisting (elastic#44483)
  FTR: fix WebDriver Actions calls (elastic#44605)
  [Code] add NodeRepositoriesService to watch new repositories on local node (elastic#44677)
  [skip-ci][Maps] Improve Maps intro page (elastic#44721)
  [Maps] Update titles and descriptions for data sources (elastic#44833)
  Types + Extract Integration Util (elastic#44433)
  Downgrade log level from info to debug for cases when we cannot handle authentication attempt. (elastic#44933)
  [Reporting] Remove Chome stdout/stderr observables, Add Browser Logger observable (elastic#44359)
  Update Jest script to output coverage (elastic#44447)
  [ftr] support --kibana-install-dir flag (elastic#44552)
  [WATCHER] Allow user to set a threshold value of 0 (elastic#44810)
  Remove injectI18n in dashboard plugin. (elastic#44580)
  [Graph] Save modal (elastic#44261)
  Use external script for the OIDC Implicit flow handler page. (elastic#44866)
  disable router prefixing with pluginId (elastic#44855)
  [SIEM] Fix bug on url + inspect functionality on hosts/hostDetails page (elastic#44671)
  [ML] File data viz limiting uploaded doc chunk size (elastic#44768)
  [code] Append go env variable 'GOCACHE' to go lsp spawn command. (elastic#44864)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 6, 2019
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into basic_sql_tem…
5d85255 
…plate

* 'master' of github.com:elastic/kibana: (91 commits)
  [APM] Make number of x ticks responsive to the plot width (elastic#44870)
  [ML] Single metric viewer: Fix top nav refresh behaviour. (elastic#44860)
  Replace TSVB timeseries charts with elastic-charts (elastic#33558)
  [TSVB][Top N aggregation] Unable to deal with negative values (elastic#43581)
  [alerting] Adds Action Type configuration support and whitelisting (elastic#44483)
  FTR: fix WebDriver Actions calls (elastic#44605)
  [Code] add NodeRepositoriesService to watch new repositories on local node (elastic#44677)
  [skip-ci][Maps] Improve Maps intro page (elastic#44721)
  [Maps] Update titles and descriptions for data sources (elastic#44833)
  Types + Extract Integration Util (elastic#44433)
  Downgrade log level from info to debug for cases when we cannot handle authentication attempt. (elastic#44933)
  [Reporting] Remove Chome stdout/stderr observables, Add Browser Logger observable (elastic#44359)
  Update Jest script to output coverage (elastic#44447)
  [ftr] support --kibana-install-dir flag (elastic#44552)
  [WATCHER] Allow user to set a threshold value of 0 (elastic#44810)
  Remove injectI18n in dashboard plugin. (elastic#44580)
  [Graph] Save modal (elastic#44261)
  Use external script for the OIDC Implicit flow handler page. (elastic#44866)
  disable router prefixing with pluginId (elastic#44855)
  [SIEM] Fix bug on url + inspect functionality on hosts/hostDetails page (elastic#44671)
  ...
spalger pushed a commit that referenced this pull request Sep 6, 2019
[ftr] support --kibana-install-dir flag (#44552) (#44935)
124f91c 
* [ftr] support --kibana-install-dir flag

* improve spacing/style of success message

* Remove unused var
spalger pushed a commit that referenced this pull request Sep 6, 2019
[ftr] support --kibana-install-dir flag (#44552) (#44936)
9f6f417 
* [ftr] support --kibana-install-dir flag

* improve spacing/style of success message

* Remove unused var
@spalger
Copy link
Contributor Author

spalger commented Sep 6, 2019

7.x/7.5: 124f91c
7.4: 9f6f417

@spalger spalger deleted the implement/ftr-kibana-install-path branch September 6, 2019 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mw-ding mw-ding mw-ding approved these changes

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.4.0 v7.5.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@spalger @elasticmachine @mw-ding