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

Migrate Snapshots to Cypress native screenshots #74

Merged
merged 6 commits into from
Jan 31, 2024

Conversation

aabidsofi19
Copy link
Contributor

@aabidsofi19 aabidsofi19 commented Jan 3, 2024

Notes for Reviewers

This PR fixes

This pr migrates the snapshot service to use cypress's screenshoting feature .
reason :

  • Cytoscape's export to png doesnt captures only the canvas layer , which prevents from including relations and elements that are rendered in separate layers into the snapshots

// Notes for maintainers :

  • we are now taking separate light mode and dark mode snapshots
  • the snapshots are in the screentshots directory rather than downloads
  • here is the relative path inside that directory :
const snapshotPath = (designId, theme) => {
  const date = new Date();
  return `MeshMap-${designId}-${date.toDateString()}/${date.toLocaleTimeString()}-${theme}`;
};
  • Yes, I signed my commits.

@leecalcote
Copy link
Contributor

Since this is a filename, we will actually lowercase meshmap. Also, I prefer that we don’t use this name understanding that it may change.

@leecalcote
Copy link
Contributor

There are two different types of snapshots right now, stored in two different folders…

Copy link
Contributor

@MUzairS15 MUzairS15 left a comment

Choose a reason for hiding this comment

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

I have opened a PR in the provider to support dark and light mode screenshots.
We require to verify this, when the screenshots are being generated for all cases

  1. Catalog snapshots
  2. Snapshots on PRs
    we will require to update the add_catalog workflow and the logic in meshery.io.

@aabidsofi19
Copy link
Contributor Author

@leecalcote in the same folder :
image

@leecalcote
Copy link
Contributor

It would be good to walk through this change on tomorrow's meeting, @aabidsofi19.

@RipulHandoo, you might like to join that meeting.

@RipulHandoo
Copy link

@leecalcote Sir. I'd love to join the meeting tomorrow, but as per my understanding and the community calendar, our next meeting is scheduled for Monday, 8th Jan 2024. Please let me know if there's any change or if you meant a different meeting.

Copy link
Contributor

@theBeginner86 theBeginner86 left a comment

Choose a reason for hiding this comment

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

Will you please ensure that these e2e tests don't fail. https://github.com/layer5labs/meshmap-snapshot/actions/runs/7397423535/job/20124507958?pr=74

Since you have updated the download location so please consider updating the tests accordingly. if required 👍
https://github.com/layer5labs/meshmap-snapshot/blob/master/.github/workflows/file-path-build-test.yml
https://github.com/layer5labs/meshmap-snapshot/blob/master/.github/workflows/url-upload-test.yml

Or update the action.yml to look in the new directory.

@aabidsofi19
Copy link
Contributor Author

Updated the meshery cloud and meshery.io catalogs :

meshery/meshery.io#1593

Signed-off-by: MUzairS15 <[email protected]>
Signed-off-by: MUzairS15 <[email protected]>
@MUzairS15 MUzairS15 merged commit b2d99a3 into master Jan 31, 2024
1 of 5 checks passed
@MUzairS15 MUzairS15 deleted the cypress-native-snapshots branch January 31, 2024 14:40
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