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.

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

NP licensing add functional tests #53002

Merged
merged 14 commits into from
Dec 19, 2019

Conversation

mshustov
Copy link
Contributor

@mshustov mshustov commented Dec 13, 2019

Summary

This PR:

  • Adds core_provider_plugin, providing access to the platform API in the browser.
  • Updates platform test to use core_provider_plugin instead of extending a global window object.
  • Adds an integration test for the client-side licensing plugin. This test is placed in functional tests because we need a way to control the elasticsearch state.
  • Separates integration tests for the licensing plugin & legacy xpack plugin to simplify further cleanup.
  • Creates 3 different configs for each test suit. Since elasticsearch supports downgrading license and running a trial only once.

I used the LP plugin to have access to all plugin contracts. It will stop working when we drop LP support. We will need to find a way to test a plugin + a combination of plugins in the integration tests. Requires a separate issue

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@mshustov mshustov added chore Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.6.0 labels Dec 16, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@mshustov mshustov changed the title Np licensing functional tests NP licensing add functional tests Dec 16, 2019
@mshustov mshustov marked this pull request as ready for review December 16, 2019 12:10
@mshustov mshustov requested a review from a team December 16, 2019 12:10
@elasticmachine elasticmachine mentioned this pull request Dec 16, 2019
12 tasks
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

We will need to find a way to test a plugin + a combination of plugins in the integration tests

A test plugin per scenario that have the plugin it needs to test/interact with as dependencies would not work?

test/plugin_functional/config.js Outdated Show resolved Hide resolved
Comment on lines +13 to +14
// MUST BE LAST! CHANGES LICENSE TYPE!
loadTestFile(require.resolve('./updates'));
Copy link
Contributor

Choose a reason for hiding this comment

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

🙈 Wonder what we could actually do without changing the whole test runner mechanism...

// this call enforces signature check to detect license update
// and causes license re-fetch
await window.np.setup.core.http.get('/');
await window.np.testUtils.delay(100);
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: What is the upside of exposing that kind of static utils from the globally exposed np instead of static import?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean this?

import { delay } from '...';

await browser.executeAsync(async (cb: Function) => {
  await window.np.setup.core.http.get('/');
  await delay(100);
  ...
});

Selenium serializes a function passed into browser.executeAsync(fn) to run it in a browser. Any closures will be lost.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yea right. That's indeed an excellent answer.

x-pack/test/licensing_plugin/public/updates.ts Outdated Show resolved Hide resolved
x-pack/test/licensing_plugin/scenario.ts Outdated Show resolved Hide resolved
@mshustov
Copy link
Contributor Author

A test plugin per scenario that have the plugin it needs to test/interact with as dependencies would not work?

It would work unless we need to change the elasticsearch state. In theory, we can call elasticsearch API from a plugin as well if we make the security model available in the browser as well. https://github.com/elastic/kibana/pull/53002/files#diff-286f825e99754fbdf390bfeedaff15f6R21

@mshustov
Copy link
Contributor Author

@pgayvallet updated

@mshustov mshustov requested a review from pgayvallet December 17, 2019 13:22
Comment on lines 37 to 39
await browser.execute(() => {
const { setup } = ((window as unknown) as CoreProvider).__coreProvider;
return setup.plugins.core_plugin_b.sayHi();
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel bad about this now that I know that vscode doesn't behave as IDEA regarding declare statement. This is indeed less elegant that the previous solution. We should decide with the team if we want to 'allow' these declare statements for extensions that are not necessarily accessible from everywhere in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I will revert this change

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@mshustov mshustov merged commit a4c5b13 into elastic:master Dec 19, 2019
@mshustov mshustov deleted the np-licensing-functional-tests branch December 19, 2019 09:59
mshustov added a commit to mshustov/kibana that referenced this pull request Dec 19, 2019
* fix comment

* introduce core provider plugin for integration tests

* platform functional tests use core_provider_plugin for testing

* add 3 scenario for licensing plugins: server, client, legacy

* remove unused code

* run all licensing_plugin tests on CI

* remove duplicated config

* address comments

* declare global type for core provider
mshustov added a commit that referenced this pull request Dec 19, 2019
* fix comment

* introduce core provider plugin for integration tests

* platform functional tests use core_provider_plugin for testing

* add 3 scenario for licensing plugins: server, client, legacy

* remove unused code

* run all licensing_plugin tests on CI

* remove duplicated config

* address comments

* declare global type for core provider
mshustov added a commit that referenced this pull request Dec 19, 2019
mshustov added a commit that referenced this pull request Dec 19, 2019
mshustov added a commit that referenced this pull request Dec 19, 2019
mshustov added a commit that referenced this pull request Dec 19, 2019
mshustov added a commit to mshustov/kibana that referenced this pull request Dec 19, 2019
* fix comment

* introduce core provider plugin for integration tests

* platform functional tests use core_provider_plugin for testing

* add 3 scenario for licensing plugins: server, client, legacy

* remove unused code

* run all licensing_plugin tests on CI

* remove duplicated config

* address comments

* declare global type for core provider
mbondyra added a commit to mbondyra/kibana that referenced this pull request Dec 19, 2019
…f github.com:mbondyra/kibana into IS-51910_share-lens-change-index-pattern-in-discover

* 'IS-51910_share-lens-change-index-pattern-in-discover' of github.com:mbondyra/kibana:
  [Discover] Remove angular field filter template code (elastic#53513)
  [APM] Improve table and other panel loading states (elastic#53459)
  Security/Spaces - cleanup react warnings (elastic#53287)
  Revert "NP licensing add functional tests (elastic#53002)" (elastic#53577)
  NP licensing add functional tests (elastic#53002)
  fix onLicenseInfoChange callback to be called on update (elastic#53559)
  Document how to extend request handler context (elastic#53271)
mshustov added a commit that referenced this pull request Dec 24, 2019
* NP licensing add functional tests (#53002)

* fix comment

* introduce core provider plugin for integration tests

* platform functional tests use core_provider_plugin for testing

* add 3 scenario for licensing plugins: server, client, legacy

* remove unused code

* run all licensing_plugin tests on CI

* remove duplicated config

* address comments

* declare global type for core provider

* remove potentially dangerous operation

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
mshustov added a commit to mshustov/kibana that referenced this pull request Dec 24, 2019
* NP licensing add functional tests (elastic#53002)

* fix comment

* introduce core provider plugin for integration tests

* platform functional tests use core_provider_plugin for testing

* add 3 scenario for licensing plugins: server, client, legacy

* remove unused code

* run all licensing_plugin tests on CI

* remove duplicated config

* address comments

* declare global type for core provider

* remove potentially dangerous operation

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
mshustov added a commit that referenced this pull request Dec 24, 2019
* NP licensing add functional tests (#53002)

* fix comment

* introduce core provider plugin for integration tests

* platform functional tests use core_provider_plugin for testing

* add 3 scenario for licensing plugins: server, client, legacy

* remove unused code

* run all licensing_plugin tests on CI

* remove duplicated config

* address comments

* declare global type for core provider

* remove potentially dangerous operation

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Jan 8, 2020
* NP licensing add functional tests (elastic#53002)

* fix comment

* introduce core provider plugin for integration tests

* platform functional tests use core_provider_plugin for testing

* add 3 scenario for licensing plugins: server, client, legacy

* remove unused code

* run all licensing_plugin tests on CI

* remove duplicated config

* address comments

* declare global type for core provider

* remove potentially dangerous operation

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported chore Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants