Skip to content

[Scout] add maps test#204607

Merged
wayneseymour merged 72 commits intoelastic:mainfrom
wayneseymour:add-scout-maps-tests
Jan 30, 2025
Merged

[Scout] add maps test#204607
wayneseymour merged 72 commits intoelastic:mainfrom
wayneseymour:add-scout-maps-tests

Conversation

@wayneseymour
Copy link
Copy Markdown
Contributor

@wayneseymour wayneseymour commented Dec 17, 2024 •
edited by kibanamachine
Loading

Summary

Add MapsPage to the scout core to be re-used by others.
Add rudimentary docs to show how to run these tests.
Added a waitForRender method.
Add test id.

@wayneseymour wayneseymour self-assigned this Dec 17, 2024
@wayneseymour wayneseymour added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Dec 17, 2024
dmlemeshko
dmlemeshko reviewed Dec 17, 2024
View reviewed changes
dmlemeshko
dmlemeshko reviewed Dec 17, 2024
View reviewed changes
dmlemeshko
dmlemeshko reviewed Dec 17, 2024
View reviewed changes
dmlemeshko
dmlemeshko reviewed Dec 17, 2024
View reviewed changes
dmlemeshko
dmlemeshko reviewed Dec 17, 2024
View reviewed changes
@dmlemeshko dmlemeshko requested a review from pheyos December 17, 2024 16:34
pheyos
pheyos reviewed Dec 18, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

Sharing some early observations, didn't check the tests yet.

@wayneseymour
Drop what I think is a duplicate test and use variables to assist wit…
dfc483e 
…h ambiguity regarding the kbn app wrapper, aka kibana chrome
nreese
nreese approved these changes Jan 29, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

Thanks for starting the MapsPage and demoing how to create a test in scout
code review only

packages/kbn-scout/src/playwright/page_objects/maps_page.ts Outdated
constructor(private readonly page: ScoutPage) {}

async gotoNewMap() {
await this.page.gotoApp(this.directNavigationOrigin);
Copy link
Copy Markdown
Contributor

@nreese nreese Jan 29, 2025

Choose a reason for hiding this comment

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

nit: I don't think there is value in putting maps/map into a constant in the class. In the name of "keeping simple things simple", how about just await this.page.gotoApp('maps/map');

nreese
nreese reviewed Jan 29, 2025
View reviewed changes
packages/kbn-scout/src/playwright/page_objects/maps_page.ts Outdated
constructor(private readonly page: ScoutPage) {}

async gotoNewMap() {
// see: https://github.com/elastic/kibana/pull/204607/files#r1932558888
Copy link
Copy Markdown
Contributor

@nreese nreese Jan 29, 2025

Choose a reason for hiding this comment

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

One more nit, why keep this comment? The method is very straightforward and I don't think we need to link the code back to this discussion. Its just more noise in the code base that does not provide value.

Copy link
Copy Markdown
Contributor Author

@wayneseymour wayneseymour Jan 29, 2025

Choose a reason for hiding this comment

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

To me, it's good to keep it as this "feels" like it's going to come up again in the future. But no worries, I'll remove.

Copy link
Copy Markdown
Contributor Author

@wayneseymour wayneseymour Jan 29, 2025

Choose a reason for hiding this comment

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

e26299c

@wayneseymour
Copy link
Copy Markdown
Contributor Author

wayneseymour commented Jan 30, 2025

@elasticmachine merge upstream

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jan 30, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #20 / FileDeleteButton isIcon clicking delete button opens the confirmation modal

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/scout 84 86 +2

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/scout 9 11 +2
Unknown metric groups

API count

id before after diff
@kbn/scout 112 114 +2

ESLint disabled line counts

id before after diff
maps 51 52 +1

Total ESLint disabled count

id before after diff
maps 78 79 +1

History

cc @wayneseymour

@wayneseymour wayneseymour merged commit ce19045 into elastic:main Jan 30, 2025
@wayneseymour wayneseymour deleted the add-scout-maps-tests branch January 30, 2025 11:55
@dmlemeshko dmlemeshko added v8.19.0 backport:version Backport to applied version labels and removed backport:skip This PR does not require backporting labels Feb 14, 2025
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 14, 2025

Starting backport for target branches: 8.x, 9.0

https://github.com/elastic/kibana/actions/runs/13333518306

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 14, 2025

💔 All backports failed

Status Branch Result
❌ 8.x Backport failed because of merge conflicts
❌ 9.0 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 204607

Questions ?

Please refer to the Backport tool documentation

dmlemeshko pushed a commit to dmlemeshko/kibana that referenced this pull request Feb 15, 2025
@wayneseymour
[Scout] add maps test (elastic#204607)
0dbaec9 
## Summary

Add MapsPage to the scout core to be re-used by others.
Add rudimentary docs to show how to run these tests.
Added a `waitForRender` method.
Add test id.

---------

Co-authored-by: Robert Oskamp <traeluki@gmail.com>
Co-authored-by: Nick Partridge <nicholas.partridge@elastic.co>
Co-authored-by: Nick Partridge <nick.ryan.partridge@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Dzmitry Lemechko <dzmitry.lemechko@elastic.co>
(cherry picked from commit ce19045)

# Conflicts:
#	.buildkite/scripts/steps/functional/scout_ui_tests.sh
#	.github/CODEOWNERS
#	x-pack/platform/plugins/shared/maps/tsconfig.json
@dmlemeshko dmlemeshko mentioned this pull request Feb 15, 2025
Merged
@dmlemeshko
Copy link
Copy Markdown
Contributor

dmlemeshko commented Feb 15, 2025

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

This was referenced Feb 15, 2025
Merged
Merged
Merged
Merged
dmlemeshko added a commit that referenced this pull request Feb 17, 2025
@dmlemeshko @wayneseymour
[8.x] [Scout] add maps test (#204607) (#211350)
a4e5dab 
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Scout] add maps test
(#204607)](#204607)

<!--- Backport version: 9.6.4 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT
[{"author":{"name":"Tre","email":"wayne.seymour@elastic.co"},"sourceCommit":{"committedDate":"2025-01-30T11:55:35Z","message":"[Scout]
add maps test (#204607)\n\n## Summary\r\n\r\nAdd MapsPage to the scout
core to be re-used by others.\r\nAdd rudimentary docs to show how to run
these tests.\r\nAdded a `waitForRender` method.\r\nAdd test
id.\r\n\r\n---------\r\n\r\nCo-authored-by: Robert Oskamp
<traeluki@gmail.com>\r\nCo-authored-by: Nick Partridge
<nicholas.partridge@elastic.co>\r\nCo-authored-by: Nick Partridge
<nick.ryan.partridge@gmail.com>\r\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>\r\nCo-authored-by: Dzmitry
Lemechko
<dzmitry.lemechko@elastic.co>","sha":"ce1904533d46e3b1e02d0d177fe4503ad0e495df","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:version","test:scout","v9.1.0","v8.19.0"],"title":"[Scout]
add maps
test","number":204607,"url":"https://github.com/elastic/kibana/pull/204607","mergeCommit":{"message":"[Scout]
add maps test (#204607)\n\n## Summary\r\n\r\nAdd MapsPage to the scout
core to be re-used by others.\r\nAdd rudimentary docs to show how to run
these tests.\r\nAdded a `waitForRender` method.\r\nAdd test
id.\r\n\r\n---------\r\n\r\nCo-authored-by: Robert Oskamp
<traeluki@gmail.com>\r\nCo-authored-by: Nick Partridge
<nicholas.partridge@elastic.co>\r\nCo-authored-by: Nick Partridge
<nick.ryan.partridge@gmail.com>\r\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>\r\nCo-authored-by: Dzmitry
Lemechko
<dzmitry.lemechko@elastic.co>","sha":"ce1904533d46e3b1e02d0d177fe4503ad0e495df"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/204607","number":204607,"mergeCommit":{"message":"[Scout]
add maps test (#204607)\n\n## Summary\r\n\r\nAdd MapsPage to the scout
core to be re-used by others.\r\nAdd rudimentary docs to show how to run
these tests.\r\nAdded a `waitForRender` method.\r\nAdd test
id.\r\n\r\n---------\r\n\r\nCo-authored-by: Robert Oskamp
<traeluki@gmail.com>\r\nCo-authored-by: Nick Partridge
<nicholas.partridge@elastic.co>\r\nCo-authored-by: Nick Partridge
<nick.ryan.partridge@gmail.com>\r\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>\r\nCo-authored-by: Dzmitry
Lemechko
<dzmitry.lemechko@elastic.co>","sha":"ce1904533d46e3b1e02d0d177fe4503ad0e495df"}},{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Tre <wayne.seymour@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nreese nreese nreese approved these changes

@dolaru dolaru dolaru left review comments

@pheyos pheyos pheyos approved these changes

@dmlemeshko dmlemeshko dmlemeshko approved these changes

@jbudz jbudz jbudz approved these changes

@nickofthyme nickofthyme nickofthyme approved these changes

Assignees

@wayneseymour wayneseymour

Labels

backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes test:scout v8.19.0 v9.0.0 v9.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@wayneseymour @elasticmachine @kibanamachine @dmlemeshko @nreese @dolaru @pheyos @jbudz @nickofthyme