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

[Reporting/Test] move functional tests to apps #64368

Merged
merged 9 commits into from
Apr 27, 2020
Merged

[Reporting/Test] move functional tests to apps #64368

merged 9 commits into from
Apr 27, 2020

Conversation

tsullivan
Copy link
Member

Summary

Reporting functional tests used a custom config and location, which seems to be only because it was one of the first X-Pack features that had functional tests. This PR moves the tests for Reporting integration into the test areas of the apps that integrate with Reporting, which puts Reporting in a similar arrangement as other X-Pack services such as feature controls, preserve-url, etc.

Checklist

Delete any items that are not applicable to this PR.

@tsullivan
Squashed commit of the following:
40d735a 
commit 5953089c03bea6b2d091f7723fea25bb1c210ee8
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 18:29:55 2020 -0700

    move tests to apps

commit 39adeaa
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 17:49:20 2020 -0700

    update archive with better dashboard

commit 55b6007
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 17:16:53 2020 -0700

    fix the refactoring bugs

commit 11aff10
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 17:16:28 2020 -0700

    remove unused fixtuers

commit 05c3381
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 16:37:36 2020 -0700

    Start of refactoring

commit b63c182
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 16:32:50 2020 -0700

    Todo comments

commit 1e0105e
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 14:31:58 2020 -0700

    revert unrelated change

commit 206fd14
Merge: 0d4c2ad 8343064
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 14:28:45 2020 -0700

    Merge branch 'master' into reporting/test-better

commit 0d4c2ad
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Wed Apr 8 10:41:19 2020 -0700

    fix ts

commit 890128c
Merge: d9ce402 3598b8c
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Wed Apr 8 10:31:09 2020 -0700

    Merge branch 'master' into reporting/test-better

commit d9ce402
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Tue Apr 7 08:31:58 2020 -0700

    [Reporting] convert all server unit tests to TypeScript
@tsullivan tsullivan added release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v8.0.0 Team:Reporting Services labels Apr 23, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

tsullivan
tsullivan commented Apr 23, 2020
View reviewed changes
x-pack/test/reporting/README.md
@@ -1,153 +0,0 @@
## The Reporting Tests
Copy link
Member Author

@tsullivan tsullivan Apr 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This README has a lot of information, but I think the only useful part is about how to update the baseline snapshots.

The previous steps for doing so were a bit complicated - I've updated the snapshot when I switched the tests to use an archived dashboard, and it was way easier than what the README said. The simplified instructions are moved out to x-pack/test/functional/apps/dashboard/reporting/README.md -- just download the report and save it in the reports/baselines folder. Easy!

As of 8.0, Reporting no longer supports backwards-compatibility URLs, so all that info was obsolete.

@tsullivan tsullivan marked this pull request as draft April 24, 2020 04:37
flash1293
flash1293 reviewed Apr 24, 2020
View reviewed changes
x-pack/test/functional/apps/dashboard/reporting/README.md Outdated
@@ -0,0 +1,17 @@
## The Reporting Tests
Copy link
Contributor

Choose a reason for hiding this comment

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

As this information isn't really dashboard specific, should we but the readme in functional/reports instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

As this information isn't really dashboard specific

For now, this information is only specific to dashboards because it's the only Reporting test that is enabled for this. Other app integrations don't yet have the image comparison test that Dashboard does. When that happens, it might be OK to just copy the file to visualize/reporting or canvas/reporting.

should we but the readme in functional/reports instead?

There is no functional/reports folder - as part of this PR, we're taking away the notion that Reporting is a standalone app.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant keeping the folder just for the readme so people can find it more easily, but copying seems even better 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the follow-up :)

flash1293
flash1293 reviewed Apr 24, 2020
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

In general fine with this change, left two little remarks about documentation

@tsullivan tsullivan marked this pull request as ready for review April 27, 2020 16:51
@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@tsullivan tsullivan merged commit 4c460b8 into elastic:master Apr 27, 2020
@tsullivan tsullivan deleted the reporting/fn-test-move-to-apps branch April 27, 2020 21:25
@tsullivan tsullivan mentioned this pull request Apr 27, 2020
Merged
tsullivan added a commit to tsullivan/kibana that referenced this pull request Apr 27, 2020
@tsullivan @elasticmachine
[Reporting/Test] move functional tests to apps (elastic#64368)
9e02eb5 
* Squashed commit of the following:

commit 5953089c03bea6b2d091f7723fea25bb1c210ee8
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 18:29:55 2020 -0700

    move tests to apps

commit 39adeaa
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 17:49:20 2020 -0700

    update archive with better dashboard

commit 55b6007
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 17:16:53 2020 -0700

    fix the refactoring bugs

commit 11aff10
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 17:16:28 2020 -0700

    remove unused fixtuers

commit 05c3381
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 16:37:36 2020 -0700

    Start of refactoring

commit b63c182
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 16:32:50 2020 -0700

    Todo comments

commit 1e0105e
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 14:31:58 2020 -0700

    revert unrelated change

commit 206fd14
Merge: 0d4c2ad 8343064
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 14:28:45 2020 -0700

    Merge branch 'master' into reporting/test-better

commit 0d4c2ad
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Wed Apr 8 10:41:19 2020 -0700

    fix ts

commit 890128c
Merge: d9ce402 3598b8c
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Wed Apr 8 10:31:09 2020 -0700

    Merge branch 'master' into reporting/test-better

commit d9ce402
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Tue Apr 7 08:31:58 2020 -0700

    [Reporting] convert all server unit tests to TypeScript

* fix imports and readmes

* remove not-needed readme

* remove extra info from readme

* correct some comments

* log the error that was caught

* fix config path in readme

* fix readme instructions to point to updated paths

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@tsullivan tsullivan mentioned this pull request Apr 27, 2020
Merged
1 task
tsullivan added a commit that referenced this pull request Apr 28, 2020
@tsullivan @elasticmachine
[7.x] [Reporting/Test] move functional tests to apps (#64368) (#64590)
5b6028a 
* [Reporting/Test] move functional tests to apps (#64368)

* Squashed commit of the following:

commit 5953089c03bea6b2d091f7723fea25bb1c210ee8
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 18:29:55 2020 -0700

    move tests to apps

commit 39adeaa
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 17:49:20 2020 -0700

    update archive with better dashboard

commit 55b6007
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 17:16:53 2020 -0700

    fix the refactoring bugs

commit 11aff10
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 17:16:28 2020 -0700

    remove unused fixtuers

commit 05c3381
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 16:37:36 2020 -0700

    Start of refactoring

commit b63c182
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 16:32:50 2020 -0700

    Todo comments

commit 1e0105e
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 14:31:58 2020 -0700

    revert unrelated change

commit 206fd14
Merge: 0d4c2ad 8343064
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Thu Apr 9 14:28:45 2020 -0700

    Merge branch 'master' into reporting/test-better

commit 0d4c2ad
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Wed Apr 8 10:41:19 2020 -0700

    fix ts

commit 890128c
Merge: d9ce402 3598b8c
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Wed Apr 8 10:31:09 2020 -0700

    Merge branch 'master' into reporting/test-better

commit d9ce402
Author: Timothy Sullivan <tsullivan@elastic.co>
Date:   Tue Apr 7 08:31:58 2020 -0700

    [Reporting] convert all server unit tests to TypeScript

* fix imports and readmes

* remove not-needed readme

* remove extra info from readme

* correct some comments

* log the error that was caught

* fix config path in readme

* fix readme instructions to point to updated paths

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

* revert

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This was referenced Jul 8, 2020
Closed
Closed
rylnd added a commit to rylnd/kibana that referenced this pull request Dec 22, 2020
@rylnd
Fix broken link
468d2c0 
This file was moved as part of elastic#64368.
rylnd added a commit that referenced this pull request Jan 5, 2021
@rylnd @dhurley14
[Security Solution] Cypress documentation updates (#86835)
1363a2a 
* Update/refactor some cypress documentation

* Fixes some whitespace/grammar/typos
* Condenses the explanation/instructions for the different modes of
  execution

* Condense Artifacts section

This is a big sprawling file; trying to cut down on the noise.

* Move test-running section to top of README

This is going to be what 90% of readers are looking for, methinks.

* Adds Security Solution's cypress suite to x-pack testing README

* Fix broken link

This file was moved as part of #64368.

* Remove broken link

This file was deleted in #67138.

* Apply suggestions from code review

Co-authored-by: Devin W. Hurley <snowmiser111@gmail.com>

* Fix typo

Co-authored-by: Devin W. Hurley <snowmiser111@gmail.com>
rylnd added a commit to rylnd/kibana that referenced this pull request Jan 5, 2021
@rylnd @dhurley14
[Security Solution] Cypress documentation updates (elastic#86835)
5a8c5d0 
* Update/refactor some cypress documentation

* Fixes some whitespace/grammar/typos
* Condenses the explanation/instructions for the different modes of
  execution

* Condense Artifacts section

This is a big sprawling file; trying to cut down on the noise.

* Move test-running section to top of README

This is going to be what 90% of readers are looking for, methinks.

* Adds Security Solution's cypress suite to x-pack testing README

* Fix broken link

This file was moved as part of elastic#64368.

* Remove broken link

This file was deleted in elastic#67138.

* Apply suggestions from code review

Co-authored-by: Devin W. Hurley <snowmiser111@gmail.com>

* Fix typo

Co-authored-by: Devin W. Hurley <snowmiser111@gmail.com>
rylnd added a commit that referenced this pull request Jan 5, 2021
@rylnd @dhurley14
[Security Solution] Cypress documentation updates (#86835) (#87266)
e83cb10 
* Update/refactor some cypress documentation

* Fixes some whitespace/grammar/typos
* Condenses the explanation/instructions for the different modes of
  execution

* Condense Artifacts section

This is a big sprawling file; trying to cut down on the noise.

* Move test-running section to top of README

This is going to be what 90% of readers are looking for, methinks.

* Adds Security Solution's cypress suite to x-pack testing README

* Fix broken link

This file was moved as part of #64368.

* Remove broken link

This file was deleted in #67138.

* Apply suggestions from code review

Co-authored-by: Devin W. Hurley <snowmiser111@gmail.com>

* Fix typo

Co-authored-by: Devin W. Hurley <snowmiser111@gmail.com>

Co-authored-by: Devin W. Hurley <snowmiser111@gmail.com>
@sophiec20 sophiec20 added the (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flash1293 flash1293 flash1293 left review comments

@joelgriffith joelgriffith joelgriffith approved these changes

@stacey-gammon stacey-gammon Awaiting requested review from stacey-gammon

@rashmivkulkarni rashmivkulkarni Awaiting requested review from rashmivkulkarni

@timroes timroes Awaiting requested review from timroes

Assignees
No one assigned
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tsullivan @elasticmachine @kibanamachine @joelgriffith @flash1293 @sophiec20