NETOBSERV-2252: Fix ConsolePlugin cypress tests #1042
Conversation
|
@oliver-smakal: This pull request references NETOBSERV-2252 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Hi @oliver-smakal. Thanks for your PR. I'm waiting for a netobserv member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test? |
|
@oliver-smakal: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test all |
|
@oliver-smakal: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
|
@oliver-smakal: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/assign @oliver-smakal |
|
/lgtm |
|
@oliver-smakal: you cannot LGTM your own PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| changeMetricType(name: string): Chainable<Element> | ||
| checkRecordField(field: string, name: string, values: string[]) | ||
| clickShowDuplicates() | ||
| } |
There was a problem hiding this comment.
All of the method signature definitions in here:
declare global {
namespace Cypress {
interface Chainable {
Seems to not be matching the signatures of the function declaratinons. For example:
Cypress.Commands.add('changeMetricType', (name) => {
cy.showAdvancedOptions();
cy.showDisplayOptions();
cy.get('#metricType-dropdown').click();
cy.get('.pf-v5-c-menu__content').contains(name).click();
cy.get('[data-layer-id="default"]').children().its('length').should('be.gte', 5);
});
has not return, but the return type of Chainable<Element> is defined in the interface.
Am I missing something or is this wrong?
There was a problem hiding this comment.
Yeah that looks like bad copy pastes 😉
There was a problem hiding this comment.
interesting, I guess our custom commands have that problem too :) , mostly from copy pasting the signatures :). We mostly use custom commands to abstract set of common of UI interactions.
thanks for spotting that.
|
/cc @jpinsonneau |
|
/cc @memodi |
|
/test plugin-cypress |
| changeMetricType(name: string): Chainable<Element> | ||
| checkRecordField(field: string, name: string, values: string[]) | ||
| clickShowDuplicates() | ||
| } |
There was a problem hiding this comment.
interesting, I guess our custom commands have that problem too :) , mostly from copy pasting the signatures :). We mostly use custom commands to abstract set of common of UI interactions.
thanks for spotting that.
|
/ok-to-test |
openshift-ci
bot
added
ok-to-test
|
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=a263aec make set-plugin-image |
Well it seems to be flaky test. Originally I tried to do it without mock data and it seemed to work for a while, hence I was trying to see why it only does not work with mock data, but after some time I figured out that the test itself is flaky. I wrote a short script to run it locally 50 times and the following is the outcome: Investigating further. |
github-actions
bot
removed
the
ok-to-test
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1042 +/- ##
==========================================
- Coverage 55.99% 53.10% -2.90%
==========================================
Files 165 205 +40
Lines 7633 11137 +3504
Branches 1294 1295 +1
==========================================
+ Hits 4274 5914 +1640
- Misses 3016 4711 +1695
- Partials 343 512 +169
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
/test plugin-cypress |
|
@jpinsonneau @memodi , the 2nd failing test should be also fixed and therefore the pr is ready for further review. As for the interface - reading though documentation, the output of any command will always be |
| //Save | ||
| cy.setupNetworkIdleTracking('GET', '/api/flow/metrics*'); | ||
| cy.get('#overview-panels-modal').contains('Save').click(); | ||
|
|
There was a problem hiding this comment.
I am curious, how this is different than simply intercepting for /api/flow/metrics request and waiting for call to happen, like we have here:
There was a problem hiding this comment.
Well, the interception you are referencing would be the easiest way to solve it, if there would be only one (or a few requests) that would be triggered.
The problem I ran into is there are like 60 request done on which it is necessary to wait. In theory, it could be implemented in a way that I count all the requests that are being done and wait for the exact amount to be finished, but I believe that could be quite easy to break if something in the implementation of how the data is fetched, which is not really a subject this test case.
So essentially this implementation waiting until the network is idle for some time. Once it is idle, it assumes the all of the data-fetching is done and would stop waiting.
There was a problem hiding this comment.
got it, thanks for the explanation.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
Description
fixes table.spec.ts tests
Dependencies
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.