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

[GUI-tests] Open only one socket connection per test scenario #9948

Merged
merged 1 commit into from
Sep 7, 2022

Conversation

saw-jan
Copy link
Member

@saw-jan saw-jan commented Jul 22, 2022

In the GUI tests, the function hasSyncStatus opens a new socket connection every time it is called. The main issue is with waitForFileOrFolderToHaveSyncStatus function which calls hasSyncStatus function rapidly until the condition is met. So, there will be many socket connection requests in a single test scenario (which may or may not be an issue for the Unix socket server).

IMO, I think we must open only a single socket connection per test scenario. So, in this PR, I have refactored the test code to only create a new socket connection if there is none and reuse the existing connection until a test scenario finishes and close the connection afterward. This way, we will open only one socket connection per scenario.

I tried to reuse a single connection for a whole test suite but that was not possible. I think the connection will be different every time we add a user to the client (I have no idea).

@saw-jan saw-jan self-assigned this Jul 22, 2022
@TheOneRing
Copy link
Member

The socket api I designed for long running connections.
The windows explroer for examples connect to the socket stays connected until either the client(the explorer) or server (the desktop client) are closed.
So I agree that you probably should keep the socket open.

@ownclouders
Copy link
Contributor

ownclouders commented Jul 22, 2022

Results for GUI-tests https://drone.owncloud.com/owncloud/client/12724/6/1

💥 The GUI tests failed.

GUI Logs: https://cache.owncloud.com/public/owncloud/client/12724/guiReportUpload/index.html

Server Logs: https://cache.owncloud.com/public/owncloud/client/12724/guiReportUpload/serverlog.log

Screenshots:

@saw-jan
Copy link
Member Author

saw-jan commented Jul 22, 2022

The socket api I designed for long running connections. The windows explroer for examples connect to the socket stays connected until either the client(the explorer) or server (the desktop client) are closed. So I agree that you probably should keep the socket open.

If the connection will get closed after closing the desktop client then I guess we won't be able to reuse a single connection for a whole test-suite. That's why I was getting trouble using single connection for a test-suite.

@saw-jan saw-jan force-pushed the preserve-socket-connection branch 3 times, most recently from 952eecd to a393d2f Compare July 25, 2022 06:34
@saw-jan
Copy link
Member Author

saw-jan commented Jul 25, 2022

I don't think the test failures with the following screen are because of the test code. We need to find out why this xfce4 login screen comes out during test.

SS

logging_out

@sonarcloud
Copy link

sonarcloud bot commented Aug 2, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@saw-jan
Copy link
Member Author

saw-jan commented Aug 4, 2022

Blocked until owncloud-ci/squish#34

@github-actions github-actions bot added the Stale label Sep 4, 2022
@saw-jan saw-jan removed the Stale label Sep 5, 2022
@sonarcloud
Copy link

sonarcloud bot commented Sep 7, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Contributor

@amrita-shrestha amrita-shrestha left a comment

Choose a reason for hiding this comment

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

Like you mentioned above using a single socket for a test suite has a problem then this changes LGTM

@amrita-shrestha amrita-shrestha merged commit 69d0e9e into master Sep 7, 2022
@delete-merged-branch delete-merged-branch bot deleted the preserve-socket-connection branch September 7, 2022 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants