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.

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

(Likely) fix flaky sidebar tests #888

Closed
wants to merge 1 commit into from
Closed

Conversation

beardhatcode
Copy link
Contributor

@beardhatcode beardhatcode commented May 2, 2021

I think this might fix the flaky test for the sidebar.

The issue was likey caused because the click to open the menu occured while the header was not visible. The act of clicking
however revealed the header without opening the menu. As a concequence, the next step does not have a menu item to
click on.

successful runs via CI (triggered by force pushes to this branch):

  1. https://dashboard.cypress.io/projects/xysa6x/runs/2555/specs
  2. https://dashboard.cypress.io/projects/xysa6x/runs/2556/specs
  3. https://dashboard.cypress.io/projects/xysa6x/runs/2557/specs
  4. https://dashboard.cypress.io/projects/xysa6x/runs/2558/specs
  5. https://dashboard.cypress.io/projects/xysa6x/runs/2559/specs

@beardhatcode beardhatcode changed the title (Naybe) fix flaky sidebar tests (Maybe) fix flaky sidebar tests May 2, 2021
@beardhatcode beardhatcode force-pushed the fix/flaky-sidebar branch 3 times, most recently from e96cad9 to 6f4fee1 Compare May 2, 2021 15:49
@beardhatcode beardhatcode changed the title (Maybe) fix flaky sidebar tests (Likely) fix flaky sidebar tests May 2, 2021
@beardhatcode beardhatcode requested a review from skjnldsv May 2, 2021 17:44
@beardhatcode
Copy link
Contributor Author

@skjnldsv, may I draw your attention to this PR before the others, It will likely make the CI of the other tests more deterministic

@skjnldsv
Copy link
Member

skjnldsv commented May 2, 2021

We're supposed to hide disable the hiding during testing 🙈

:clear-view-delay="isTesting ? -1 : 5000 /* prevent cypress timeouts */"

@beardhatcode
Copy link
Contributor Author

Odd, that is not working in my local cypress, but maybe I did something wrong. I throttled cypresses download speed and noticed this problem.
Hmm, maybe the mousemove triggers something else then? Or isTesting is undefined? (Or this PR just works accidentally 😛 )

@skjnldsv
Copy link
Member

skjnldsv commented May 3, 2021

The isTesting variable is definitely working, since I see the debug line in the cypress building tests.

viewer/webpack.js

Lines 13 to 15 in 9d33800

if (isTesting) {
console.debug('TESTING MODE ENABLED')
}

@skjnldsv
Copy link
Member

skjnldsv commented May 3, 2021

I guess there is something else, let me check

@skjnldsv
Copy link
Member

skjnldsv commented May 3, 2021

Yep, the UI doesn't hide when testing is senbaled, so it works.
I think the failing test are linkd to the Sidebar Opening check.

Because there is no way to currently wait for the Sidebar to be done opening, we have a weird setTimeout, but sometimes it fails 🤔

@skjnldsv
Copy link
Member

skjnldsv commented May 3, 2021

@beardhatcode nextcloud-libraries/nextcloud-vue#1914

Then we could have this into the Sidebar app and wait for proper events to either finish the Promise
https://github.com/nextcloud/server/blob/3c172ce600503faa1be653fe4f85baa9715d19ef/apps/files/src/views/Sidebar.vue#L382-L416

Or add a dedicated callback method :)

@beardhatcode beardhatcode force-pushed the fix/flaky-sidebar branch 3 times, most recently from 8555a38 to 65dcf9c Compare May 23, 2021 17:11
@beardhatcode
Copy link
Contributor Author

Here is a video that shows that the sidebar is really not working in some cases:
https://dashboard.cypress.io/projects/xysa6x/runs/2577/test-results/b71e2024-b304-4b4c-8b42-ec7cb70f91c8

@beardhatcode beardhatcode force-pushed the fix/flaky-sidebar branch 3 times, most recently from 3b595fc to 6641c65 Compare May 23, 2021 19:29
Signed-off-by: Robbert Gurdeep Singh <git@beardhatcode.be>
@beardhatcode
Copy link
Contributor Author

beardhatcode commented May 23, 2021

Ok, so it appears that something is going wrong with clicking on the "Open Sidebar" button. I assume this is caused by the transition (but it would be weird if that were the case), so I added some waits, and now it seems to work reproducibly:

https://dashboard.cypress.io/projects/xysa6x/runs/2581/overview
https://dashboard.cypress.io/projects/xysa6x/runs/2582/overview
https://dashboard.cypress.io/projects/xysa6x/runs/2583/overview

The odd thing is: if this works, why doesn't the download spec fail, it does basically the same with no problem:

it('Download the image', function() {
// open the menu
cy.get('body > .viewer .modal-header button.action-item__menutoggle').click()
// download the file
cy.get('.action-link__icon.icon-download').click()
})

@beardhatcode
Copy link
Contributor Author

@skjnldsv, could you take another look at this?

@skjnldsv
Copy link
Member

@beardhatcode nextcloud/nextcloud-vue#1914

Then we could have this into the Sidebar app and wait for proper events to either finish the Promise
nextcloud/server@3c172ce/apps/files/src/views/Sidebar.vue#L382-L416

First pr has been merged and backported

@beardhatcode
Copy link
Contributor Author

But that will not fix the issue @skjnldsv. The sidebar really doesn't open, and that is why the test fails.

See the video in: https://dashboard.cypress.io/projects/xysa6x/runs/2577/test-results/b71e2024-b304-4b4c-8b42-ec7cb70f91c8

@blizzz blizzz added this to the Nextcloud 25 milestone Apr 21, 2022
@skjnldsv skjnldsv closed this May 3, 2022
@skjnldsv skjnldsv deleted the fix/flaky-sidebar branch May 3, 2022 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants