Add test scaffolding for vscode-graphql-syntax (option 2)

[AaronMoat]

Resolves: #2343

Alternative to #3267 using much more custom code, but I think it gives us more control. Very keen to see what people think!

[changeset-bot]

⚠️ No Changeset found

Latest commit: 1d7d118

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[acao]

@AaronMoat oh this is very nice, and very easy to read!

In general, writing tests for anything vscode related will involve complicated test tooling, but this seems like it will really do the trick!

[acao]

I wonder why this doesn't seem to trigger a codecov report?

[AaronMoat]

I guess it did? but there's no new "code" tested by this change technically, "just" the package.json (from the eyes of coverage)

[acao]

weird, i don't see the comment for some reason ! but yes looking at the diff, no new coverage data

I guess it makes sense, as istanbul (or whatever jest uses now) wouldn't have a way to instrument JSON for coverage in any way it could recognize.

That's ok though, coverage data is only a rough metric for code quality, I think we can all agree how important this test suite is!

[AaronMoat]

@acao no comment - I was clicking through to the github actions 'details' :)

[AaronMoat]

So @acao seems you're a fan of this approach over the other one. Is that right - should I close off the other one and prefer this? I might flesh it out with a few more test examples.

Do you prefer inline snapshots or proper snapshots?

There's a few options

1. expect(fileName).toMatchSnapshot()
2. expect(fileName).toMatchInlineSnapshot(stuff)
3. expect(/* GraphQL */"blah").toMatchSnapshot()
4. expect(/* GraphQL */"blah").toMatchInlineSnapshot(stuff)

Any thoughts on that as well?

[acao]

all very good questions! I'm not sure I am ready to decide yet, and I'd be interested in what my colleagues think as well

[dimaMachina]

@AaronMoat Nicely job, thanks for putting effort into this. I prefer this PR rather than the first one. (I found here more readable)

But to be honest it's not exactly what I am searching for. The goal of this PR is to test tokenizing right? We can still keep it, it's a good job. (let me know if I am right in terms I am still new to this project and still learning 😅)

But I have in mind to have image syntax highlighting snapshots tests, and not just snapshots tests. I wanna test the right syntax highlighting and throw an error when % of pixels have a difference.

I was inspired by these image tests in satori https://github.com/vercel/satori/tree/main/test/image_snapshots
and I added tests for our open graph images https://github.com/the-guild-org/docs/blob/main/packages/og-image/src/__image_snapshots__/handler-test-ts-packages-og-image-src-handler-test-ts-handler-should-align-title-and-have-container-padding-1-snap.png and it fails (sometimes) when a new version of satori is out.

[dimaMachina]

There's a few options

expect(fileName).toMatchSnapshot()
expect(fileName).toMatchInlineSnapshot(stuff)
expect(/* GraphQL /"blah").toMatchSnapshot()
expect(/ GraphQL */"blah").toMatchInlineSnapshot(stuff)

I prefer the first one expect(fileName).toMatchSnapshot()

• snapshots can be long
• the same filename we can reuse to test mentioned previously image snapshots tests

[acao]

@B2o5T great idea! I've built some custom image snapshot tooling before, for a geospatial project. that might be the most readable option. the underlying libraries for image diffing used by percy and these jest plugins are quite surprisingly simple and powerful

the problem herein is that we have to find a way to render an image of the syntax highlighting in the first place. monaco for example uses a different grammar than vscode, but I'm sure we can find a library to render vscode grammars somewhere

[dimaMachina]

that might be the most readable option.

exactly! I found image snapshots, the most maintainable option

the problem herein is that we have to find a way to render an image of the syntax highlighting in the first place

you have a point, this is most trickiest part :)

[AaronMoat]

Fully agree that images would be better. The how is the big question :)

[acao]

I think I'll just build this library and a set of runners (cypress, playwright, etc) for fun 😜

Shared Steps

1. render dom with codemirror, codemirror textmate mode and load our grammars
2. domToSvG(), then

test assert

pixelmatch against each SVG, exit non 0 if pixelalmatch doesn't match, write out failing pixelmatch results with res error highlighting for the diff

snap update

save svg output

[acao]

Here is another upcoming issue, you may have mentioned already, is that there are multiple issues regarding embedded languages:
microsoft/vscode-textmate#207

and this:

Cross - grammar injections are currently not supported.

which I believe will mean that "injectionSelector" rules won't work with this approach of converting the grammars to textmate grammars

not to fret though, being able to test just our rules for graphql strings is most important, perhaps we can help contribute support for injectionSelector to vscode-textmate

update: I think I misread or the readme is out of date, I think injectionSelector support will work as of microsoft/vscode-textmate#11

[acao]

looking to carbon for inspiration on code -> image front, and I found this! haha!

[acao]

here is a nice rust library that seems to use modern stdout features to print code without a browser at all. the problem I anticipate is that this rendering will be inconsistent across various local and CI environments

https://github.com/trishume/syntect#screenshots

[acao]

after some research, vscode is one of the few web IDEs that supports injectionSelector textmate rules, so it would be very difficult to do visual snapshot testing. @AaronMoat 's approach here is probably the best approach, of using the tokenizer that vscode-texmate provides. the only potential route for rendering these grammars completely in the web I can find is to use https://github.com/CodinGame/monaco-vscode-api which is quite complex to orchestrate. if it were just the graphql grammar, it wouldn't be too hard to get image snapshot diffing, but the embedded grammars are some of the most important to test

[acao]

just added more tests cases and organized things a bit. lets go with this strategy for now so we have something to make improvements on and to use as a regression suite until we figure out a good solution for using images. i think shiki supports injection rules, which is heavily based on vscode-textmate