Skip to content

Conversation

@parkiino
Copy link
Contributor

@parkiino parkiino commented Jun 17, 2020

Summary

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@parkiino parkiino marked this pull request as ready for review June 23, 2020 22:11
@parkiino parkiino requested a review from a team as a code owner June 23, 2020 22:11
@parkiino parkiino requested a review from a team June 23, 2020 22:11
@parkiino parkiino requested a review from a team as a code owner June 23, 2020 22:11
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jun 23, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@parkiino parkiino added release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Management v7.9.0 v8.0.0 labels Jun 23, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-management (Team:Endpoint Management)

});

it('displays table data', async () => {
it.skip('displays table data', async () => {
Copy link
Contributor

@kevinlog kevinlog Jun 23, 2020

Choose a reason for hiding this comment

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

is this intentional?

EDIT: or is this identified as the flaky test?


// FLAKY: https://github.com/elastic/kibana/issues/63621
describe.skip('endpoint list', function () {
describe('endpoint list', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

did you run this a few times locally to make sure it's no longer flaky?

const hostDetailTitleInitial = await testSubjects.getVisibleText('hostDetailsFlyoutTitle');
// select the same host in the host list
await (await testSubjects.findAll('hostnameCellLink'))[1].click();
await sleep(500); // give page time to refresh and verify it did not change
Copy link
Contributor

Choose a reason for hiding this comment

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

this was being skipped, before right? sleep() in tests make me a bit nervous, just wanna make sure this is run enough times to ensure this isn't flaky

Copy link
Contributor

Choose a reason for hiding this comment

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

@kevinlog The entire suite is being skipped (see describe on line 17), so I think we're goo for now. @charlie-pichette will take care of fixing the flakiness


export const services = {
...kibanaCommonServices,
supertest: kibanaApiIntegrationServices.supertest,
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you make these changes or was it from a master merge?

If you did - would like to know why we had to change the common services. was this because the API tests needed supertest? even then - adding it here impacts everyone, so not sure we want to do it this way

Copy link
Contributor Author

@parkiino parkiino Jun 24, 2020

Choose a reason for hiding this comment

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

it was a master merge. i just moved this file out of test/api_integrations/services and then imported this file into the test/api_integrations/services/index hopefully thats ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to clarify, i needed to add this import since the ingest manager service needs supertest after the merge from master. I moved the file from api_integration/services/ingest_manager to common/services/ingest_manager since api_integration is using ingest and so is security_solution_endpoint. i could also just add that import directly to the ingest_manager service file, if you think thats better

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you - now I understand. I guess for Endpoint UI Functional, we did not have to do that because we import the services from x-pack/test/functional/ which (my guess) is already brings that service in.
Anyway 🙏

const hostDetailTitleInitial = await testSubjects.getVisibleText('hostDetailsFlyoutTitle');
// select the same host in the host list
await (await testSubjects.findAll('hostnameCellLink'))[1].click();
await sleep(500); // give page time to refresh and verify it did not change
Copy link
Contributor

Choose a reason for hiding this comment

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

@kevinlog The entire suite is being skipped (see describe on line 17), so I think we're goo for now. @charlie-pichette will take care of fixing the flakiness

const agentId = policyInfo.agentConfig.id;
const datasourceId = policyInfo.datasource.id;

await pageObjects.common.navigateToApp('ingestManager', {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we spoke about this, and... I might be changing my mind 😃

I'm thinking that we should create a method in the ingest pageObjects we have (ingest_manager_create_datasource_page.ts) named

navigateToAgentConfigEditDatasource(agentConfigId, datasourceId)

and call it from here. It will make it easier to keep all ingest related pageObjects in one place, case these move over to Ingest at a later time ++ any future changes to URLs, etc.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

Copy link
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

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

LGTM 🚢 🚀

🙏

@parkiino parkiino merged commit 1daa2f4 into elastic:master Jun 25, 2020
@parkiino parkiino deleted the task/endpoint-list-tests branch June 25, 2020 15:10
parkiino added a commit to parkiino/kibana that referenced this pull request Jun 25, 2020
endpoint func tests for endpoint details to ingest, edit datasource to policy, bug fix for security link
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 25, 2020
* master: (90 commits)
  [Encrypted Saved Objects] Adds support for migrations in ESO (elastic#69513)
  [SIEM] Replace WithSource with useWithSource hook (elastic#68722)
  [Endpoint]EMT-451: add ability to filter endpoint metadata based on presence of unenrolled events (elastic#69708)
  rename old siem kibana config to securitySolution (elastic#69874)
  Remove unused Resolver code (elastic#69914)
  [Observability] Fixing dynamic return type based on the appName (elastic#69894)
  [SECURITY SOLUTION][INGEST] Task/endpoint list tests (elastic#69419)
  Fixes special clicks and 3rd party icon sizes in nav (elastic#69767)
  [APM] Catch annotations index permission error and log warning (elastic#69881)
  [Endpoint][Ingest Manager] minor code cleanup (elastic#69844)
  [Logs UI] Logs ui context menu (elastic#69915)
  Index pattern serialize and de-serialize (elastic#68844)
  [QA] Unskip functional tests (elastic#69760)
  [SIEM][Detection Engine] - Update DE to work with new exceptions schema (elastic#69715)
  Fixes elastic#69639: Ignore url.url fields above 2048 characters (elastic#69863)
  PR: Provide limit warnings to user when API limits are reached. (elastic#69590)
  [Maps] Remove broken button (elastic#69853)
  Makes usage collection methods available on start (elastic#69836)
  [SIEM][CASE] Improve Jira's labelling (elastic#69892)
  [Logs UI] Access ML via the programmatic plugin API (elastic#68905)
  ...
parkiino added a commit that referenced this pull request Jun 25, 2020
endpoint func tests for endpoint details to ingest, edit datasource to policy, bug fix for security link
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 26, 2020
* master:
  [Encrypted Saved Objects] Adds support for migrations in ESO (elastic#69513)
  [SIEM] Replace WithSource with useWithSource hook (elastic#68722)
  [Endpoint]EMT-451: add ability to filter endpoint metadata based on presence of unenrolled events (elastic#69708)
  rename old siem kibana config to securitySolution (elastic#69874)
  Remove unused Resolver code (elastic#69914)
  [Observability] Fixing dynamic return type based on the appName (elastic#69894)
  [SECURITY SOLUTION][INGEST] Task/endpoint list tests (elastic#69419)
  Fixes special clicks and 3rd party icon sizes in nav (elastic#69767)
  [APM] Catch annotations index permission error and log warning (elastic#69881)
  [Endpoint][Ingest Manager] minor code cleanup (elastic#69844)
  [Logs UI] Logs ui context menu (elastic#69915)
  Index pattern serialize and de-serialize (elastic#68844)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.9.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants