+
Skip to content
This repository was archived by the owner on Aug 31, 2023. It is now read-only.

test(rome_cli): more snapshot tests #3075

Merged
merged 3 commits into from
Aug 17, 2022
Merged

Conversation

ematipico
Copy link
Contributor

Summary

This PR adds more snapshot testing to our CLI tests.
I found the issue that was preventing us from applying the snapshot testing to the rest of the tests: there some tests where we manually open a file from the file system and inspect their content; when doing so, we lock the file via .lock_arc(). This was preventing the snapshot logic to read these files unless they are freed.

I manually added a drop(file) where needed, and now we have more snapshots!

Test Plan

CI should pass

@ematipico ematipico requested a review from leops as a code owner August 17, 2022 12:01
@ematipico ematipico temporarily deployed to aws August 17, 2022 12:01 Inactive
@@ -0,0 +1,6 @@
---
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These "empty" snapshots are expected because we don't read the Termination error yet. I plan to make another PR where we read the Result of a CLI session.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 17, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 67f75b7
Status:⚡️  Build in progress...

View logs

@github-actions
Copy link

github-actions bot commented Aug 17, 2022

Copy link
Contributor

@leops leops left a comment

Choose a reason for hiding this comment

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

Since we're starting to have a lot of snapshots, it would be nice to sort them by passing the value of module_path!() to assert_cli_snapshot as a directory name for the snapshot

@ematipico ematipico temporarily deployed to aws August 17, 2022 15:51 Inactive
@ematipico ematipico force-pushed the chore/more-cli-snapshots branch from ae18c34 to a77a8c8 Compare August 17, 2022 15:57
@ematipico ematipico temporarily deployed to aws August 17, 2022 15:57 Inactive
@ematipico ematipico requested a review from a team August 17, 2022 16:10
@ematipico ematipico temporarily deployed to aws August 17, 2022 16:10 Inactive
@ematipico ematipico merged commit b999aae into main Aug 17, 2022
@ematipico ematipico deleted the chore/more-cli-snapshots branch August 17, 2022 16:15
IWANABETHATGUY pushed a commit to IWANABETHATGUY/tools that referenced this pull request Aug 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载