Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix a FilePanel test to have Percy take a snapshot of file tiles on the panel #10429

Merged
merged 8 commits into from
Apr 3, 2023
Merged

Fix a FilePanel test to have Percy take a snapshot of file tiles on the panel #10429

merged 8 commits into from
Apr 3, 2023

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Mar 22, 2023

This PR should fix the issue reported here that Percy screenshot test for FilePanel does not take a screenshot of EventTiles on FilePanel, by specifying the correct element and ensuring that they are visible before having Percy take a screenshot.

It is recommended to run Percy before merging this PR to check if it works as expected.

type: task

Signed-off-by: Suguru Hirahara [email protected]

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

This change is marked as an internal change (Task), so will not be included in the changelog.

@github-actions github-actions bot added Z-Community-PR Issue is solved by a community member's PR T-Task Refactoring, enabling or disabling functionality, other engineering tasks labels Mar 22, 2023
@andybalaam andybalaam added the X-Needs-Percy Whether to run Percy screenshot tests in Merge Queue label Mar 28, 2023
@luixxiul
Copy link
Contributor Author

luixxiul commented Mar 29, 2023

The snapshot was taken as expected.

Please note that the audio player's seek bar and the timestamp on the bottom next to the profile name are hidden. Checking whether the timestamp is rendered only on the latest updated file would be a next task (element-hq/element-web#24719).

Screenshot from 2023-03-29 13-07-53

@luixxiul luixxiul marked this pull request as ready for review March 29, 2023 04:11
@luixxiul luixxiul requested a review from a team as a code owner March 29, 2023 04:11
});

// Make the viewport tall enough to display all of the file tiles on FilePanel
cy.viewport(660, 1000);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is required to check visibility of all of the file tiles. Otherwise it will return an error on the default viewport size (1000, 600).

The viewport size should be reset after each test.

Signed-off-by: Suguru Hirahara <[email protected]>
Copy link
Contributor

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@andybalaam
Copy link
Contributor

Failed due to Percy allowance being used up. It passed previously, so merging.

@andybalaam andybalaam merged commit d3cd44d into matrix-org:develop Apr 3, 2023
@luixxiul luixxiul deleted the test-file-panel branch April 3, 2023 10:28
@luixxiul
Copy link
Contributor Author

luixxiul commented Apr 3, 2023

Thanks for reviewing!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks X-Needs-Percy Whether to run Percy screenshot tests in Merge Queue Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants