-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[RAC][Observability]: test cases for alerts pagination functional tests #112617
[RAC][Observability]: test cases for alerts pagination functional tests #112617
Conversation
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Nice that you broke out the pagination parts from the provider!
I left a few nitpick comments about naming :)
x-pack/test/observability_functional/apps/observability/alerts/workflow_status.ts
Show resolved
Hide resolved
x-pack/test/functional/services/observability/alerts/pagination.ts
Outdated
Show resolved
Hide resolved
export function ObservabilityAlertsPaginationProvider({ getService }: FtrProviderContext) { | ||
const testSubjects = getService('testSubjects'); | ||
|
||
const getPageSizeSelector = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: These properties seem to be sorted by which page element they are using, but it's still a flat list. Would it be helpful to group related properties in an object?
I'm thinking something like:
const pageSizeSelector = {
get: async () => { ...
getOrFail: async () => { ...
getOrMissing: async () => { ...
}
That would make it faster to scan which properties exist for a particular page element. A middle ground between having many small providers while still helping with some readability.
Not sure what would be done for those that only have function.
x-pack/test/observability_functional/apps/observability/alerts/pagination.ts
Outdated
Show resolved
Hide resolved
x-pack/test/observability_functional/apps/observability/alerts/pagination.ts
Outdated
Show resolved
Hide resolved
x-pack/test/observability_functional/apps/observability/alerts/pagination.ts
Outdated
Show resolved
Hide resolved
await observability.alerts.common.setWorkflowStatusFilter('open'); | ||
}); | ||
|
||
it('Does not render page size selector', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious:
- Is it needed to have a separate test for the page size and pagination buttons? From a users perspective that is one and the same state, "there are no pagination controls".
- Within the DataGrid, I'm guessing these are added as two different components but is it needed for us to also check the next button for example?
x-pack/test/observability_functional/apps/observability/alerts/pagination.ts
Outdated
Show resolved
Hide resolved
x-pack/test/observability_functional/apps/observability/alerts/pagination.ts
Outdated
Show resolved
Hide resolved
x-pack/test/observability_functional/apps/observability/alerts/pagination.ts
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: cc @mgiota |
…ts (elastic#112617) * [RAC][Observability]: test cases for alerts pagination functional tests * page size selector tests * create OPEN_ALERTS_ROWS_COUNT in workdlow status tests * add tests to check if page selector is rendered or not * reorganize tests to visible and non visible pagination controls * default rows per page test * page size selector tests * more page selector tests * write tests for pagination controls * move pagination tests to a new file * remove unused variables * reorganize observability alerts service * undo configuration change * fix workflow status tests after refactoring * clean up * pr review comments * change variable name * rewording pagination tests Co-authored-by: Kibana Machine <[email protected]>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…ts (#112617) (#113474) * [RAC][Observability]: test cases for alerts pagination functional tests * page size selector tests * create OPEN_ALERTS_ROWS_COUNT in workdlow status tests * add tests to check if page selector is rendered or not * reorganize tests to visible and non visible pagination controls * default rows per page test * page size selector tests * more page selector tests * write tests for pagination controls * move pagination tests to a new file * remove unused variables * reorganize observability alerts service * undo configuration change * fix workflow status tests after refactoring * clean up * pr review comments * change variable name * rewording pagination tests Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: mgiota <[email protected]>
Fixes #110636
I reorganized a bit observability alerts services and created a separate file for pagination as a starting point. I started moving workflow status helpers to a separate file and then got stuck a bit there and left it out (the workflowStatus provider needed to use a method (getTableCells) from another common service observability provider I created, but this didn't work). Does the proposed structure I created at least for pagination make sense?
Notes
While working on this ticket tests couldn't be run against latest Chrome. As a quick workaround I was testing it on Firefox instead by changing this line https://github.com/elastic/kibana/blob/master/x-pack/test/observability_functional/with_rac_write.config.ts#L14 to use
config.firefox.js
Issue to update Driver to v94 giggio/node-chromedriver#328