search

godotengine / godot-vscode-plugin

Godot development tools for VSCode
MIT License
1.48k stars 148 forks source link

Add snapshot tests to formatter #545

Closed sandygk closed 6 months ago

sandygk commented 7 months ago

In this PR I added 17 snapshot tests to the formatter. All the tests are based on previous and current errors detected in the formatter.

The test file automatically searches for all folders inside the snapshots folder and run a test for each. Each folder in the snapshots folder has two files in.gd and out.gd. Each generated test compares the output of running the formatter on in.gd with the expected output from out.gd.

To add a new test, we can simply create a new folder in the snapshots folder, and add two files, in.gd and out.gd for the input and expected output.

In the console, we can see a diff from the expected and the actual value that we got in case of a mismatch.

From the 17 snapshot tests added, we are passing 10 and failing 7.

A simple way to run the tests is to run: npm run compile; npm run test and check the output in the console

Calinou commented 7 months ago

A simple way to run the tests is to run: npm run compile; npm run test and check the output in the console

Can we run this on CI so we make sure tests eventually all pass over time? I'd comment out the failing tests until we can get them to work.

DaelonSuzuka commented 7 months ago

Can we run this on CI

Yeah, that's on the agenda.

so we make sure tests eventually all pass over time? I'd comment out the failing tests until we can get them to work.

I'm pretty sure they all pass right now. I was planning to add a few more test cases this weekend but I got busy.

I'll probably look at the CI stuff tonight(optimistically) and then call this PR done. Big thanks to @sandygk for setting up the snapshots.

DaelonSuzuka commented 7 months ago

I was off by a day, but the test suite now runs in CI on mac/linux/win.

@sandygk @Calinou This PR might be done. I could add more tests, but I think we're already into diminishing returns.

I was considering putting the formatter behind an experimental setting that's off by default, but I actually think it's pretty solid right now. Opinions?

sandygk commented 7 months ago

I think we could leave it as it is. After the PR is merged all the major issues with the formatter should get fixed. I also don't want to go too crazy with the tests, after all there are infinite things that can potentially break. I think as long as we keep updating the tests every time we find a bug we should be OK.