Skip to content

test: add infer-parser tests#30

Merged
fabiospampinato merged 5 commits intoprettier:mainfrom
43081j:infer-parser-test
May 13, 2025
Merged

test: add infer-parser tests#30
fabiospampinato merged 5 commits intoprettier:mainfrom
43081j:infer-parser-test

Conversation

@43081j
Copy link
Collaborator

@43081j 43081j commented Jan 31, 2025

Adds the infer-parser tests from prettier.

Also updates the runCLI util to only execute the binary once a test actually runs.

@43081j
Copy link
Collaborator Author

43081j commented Feb 2, 2025

depends on #33 now

@fabiospampinato
Copy link
Collaborator

#33 got fixed, let me know when we should be ready for merging this.

@43081j 43081j force-pushed the infer-parser-test branch from b7e80e4 to 4ef2b89 Compare March 2, 2025 20:05
@43081j
Copy link
Collaborator Author

43081j commented Mar 2, 2025

Blocked by #51

const fileRelativePath = fastRelativePath(rootPath, filePath);
//TODO: Make sure the error is syntax-highlighted when possible
if (options.check || options.write) {
if (options.check || options.write || options.dump) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

importantly , this changed

this is what makes errors now log out when you're in dump mode (the same as prettier)

@fabiospampinato fabiospampinato force-pushed the main branch 4 times, most recently from 34e0257 to f8dc396 Compare March 6, 2025 00:45
@fabiospampinato
Copy link
Collaborator

#51 was addressed, do we need anything else for this PR?

43081j added 3 commits April 25, 2025 13:55
Adds the `infer-parser` tests from prettier.

Also updates the `runCLI` util to only execute the binary once a test
actually runs.
@43081j 43081j force-pushed the infer-parser-test branch from 4ef2b89 to 7f0d9c6 Compare April 25, 2025 12:56
@43081j
Copy link
Collaborator Author

43081j commented Apr 25, 2025

this should now be good to go

I also fixed some unrelated snapshots, e.g. the version changing caused snapshots to fail so I replaced it with $VERSION

the difference to note with prettier is that we don't log errors for files which have an unknown parser. we just skip them, and show that in debug logs but not in normal output

@fabiospampinato fabiospampinato merged commit 77f8136 into prettier:main May 13, 2025
3 of 4 checks passed
@fabiospampinato
Copy link
Collaborator

Thanks!

@fisker
Copy link
Member

fisker commented May 14, 2025

Reminder: the tests fail on Windows https://github.com/prettier/prettier-cli/actions/runs/15008796631/job/42173501009 . Maybe better keep the path-serializer https://github.com/prettier/prettier/blob/fd2781d042c510642a3c155dae65c1c814172a71/tests/integration/__tests__/infer-parser.js#L4

By the way @fabiospampinato, why are we removing PR number in commit message? It's hard to find linked PR.

@fabiospampinato
Copy link
Collaborator

Reminder: the tests fail on Windows

Yeah that has to be fixed. Or were only the tests from this PR failing on Windows? I should have checked that.

By the way @fabiospampinato, why are we removing PR number in commit message? It's hard to find linked PR.

I normally just write the commit message that I would have written as if the PR didn't exist.

Do you want the PR number mentioned in the message? Or is the description fine?

@fisker
Copy link
Member

fisker commented May 14, 2025

Do you want the PR number mentioned in the message? Or is the description fine?

Anywhere is good.

@fisker
Copy link
Member

fisker commented May 14, 2025

Or were only the tests from this PR failing on Windows?

Just this one now.

@43081j 43081j deleted the infer-parser-test branch May 14, 2025 15:22
@fabiospampinato
Copy link
Collaborator

Or were only the tests from this PR failing on Windows?

Just this one now.

Oh I messed it up then 🤔 @43081j could you have a look at why that is?

@43081j
Copy link
Collaborator Author

43081j commented May 14, 2025

#67

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.

3 participants