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

[Tests-Only]Enabled screenshot of desktop on test failure #9518

Merged
merged 1 commit into from
Mar 24, 2022

Conversation

Talank
Copy link
Contributor

@Talank Talank commented Mar 15, 2022

This is how the comment looks like along with the screenshots #9518 (comment)

fixes: #9517

@Talank
Copy link
Contributor Author

Talank commented Mar 15, 2022

Results for GUI-tests https://drone.owncloud.com/owncloud/client/11072/5/1 GUI Logs: (https://cache.owncloud.com/public/owncloud/client/11072/guiReportUpload/index.html) Server Logs: (https://cache.owncloud.com/public/owncloud/client/11072/guiReportUpload/serverlog.log) Screenshots: (https://cache.owncloud.com/public/owncloud/client/11072/guiReportUpload/screenshots)

The capture and upload of screenshot is actually working, https://cache.owncloud.com/public/owncloud/client/11072/guiReportUpload/screenshots/user_changes_the_expiration_date_of_an_already_existing_public_link_for_folder_using_client-UI.png

However, we still need to find an appropriate way to show all the images (May be dumping all those screenshots each time as comment would pollute the overview of the PR. On the other hand, rendering screenshots as a web page (like we displayed the GUI test result) might be better.

@Talank Talank self-assigned this Mar 15, 2022
@Talank Talank force-pushed the GUI-test-error-screenshot branch 2 times, most recently from 7f385bb to 4847ffa Compare March 16, 2022 04:19
@Talank Talank marked this pull request as ready for review March 18, 2022 07:40
This was referenced Mar 19, 2022
Copy link
Member

@saw-jan saw-jan left a comment

Choose a reason for hiding this comment

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

LGTM

.drone.star Outdated Show resolved Hide resolved
echo "creating comment for screenshots"
echo "Screenshots:" >> $1/comments.file
for i in $(ls $1/screenshots); do
echo "- [$i](${CACHE_ENDPOINT}/${CACHE_BUCKET}/$2/$3/guiReportUpload/screenshots/$i)" >> $1/comments.file
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
echo "- [$i](${CACHE_ENDPOINT}/${CACHE_BUCKET}/$2/$3/guiReportUpload/screenshots/$i)" >> $1/comments.file
echo "- [$i](${CACHE_ENDPOINT}/${CACHE_BUCKET}/$2/$3/${GUI_TEST_REPORT_DIR}/screenshots/$i)" >> $1/comments.file

also $1 $2 $3 should be given nice names
both could happen in a different PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$GUI_TEST_REPORT_DIR is /drone/src/test/guiReportUpload. We either need to create new env variable to store the directory name guiReportUpload or we should leave it as it is, AFAIK.

And yes, we can give some nice name to all the variables, that can be done in some other PR.

@@ -93,6 +93,24 @@ def hook(context):

@OnScenarioEnd
Copy link
Member

Choose a reason for hiding this comment

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

that will create an screenshot AFTER the issue
so e.g. if the app crashes this will be the screen after the crash, meaning the app would not even be visible
Do we need (additionally?) the screenshot just before the problematic action? But that might be hard to achive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the app is not closed automatically after the crash then the screenshot is captured that shows the state of crash. For instance in #9518 (comment)

Otherwise, the only option that I can think of right now is to capture the video of test run (anyway) and then delete it if there is no test error. I can not say that can be done easily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But this could be a good enhancement. I will open an issue. May be we can try it later.

@sonarcloud
Copy link

sonarcloud bot commented Mar 24, 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

@Talank Talank merged commit 2dc0817 into 2.10 Mar 24, 2022
@delete-merged-branch delete-merged-branch bot deleted the GUI-test-error-screenshot branch March 24, 2022 05:06
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.

5 participants