feat(ci): test against nvim-treesitter highlights #169
Conversation
|
|
||
| - name: checkout scala/scala | ||
| if: ${{ runner.os != 'Windows' }} | ||
| if: ${{ runner.os == 'Linux' }} |
There was a problem hiding this comment.
Not exactly related to this but I realized we are cloning this on mac as well, but not running the smoke tests on mac. So I just switched this back to only clone on the os that we actually run it on.
There was a problem hiding this comment.
As a separate point, I think we can extract the Linux workflow as a separate thing, and on Mac/Windows just do npm run build && npm run test.
To declutter all those if conditionals.
There was a problem hiding this comment.
I can try to follow-up with another pr and clean this up.
This PR starts testing against the highlights that nvim-treesitter has defined to ensure we don't break things without warning. The general flow is as follows: - When on linux we clone nvim-treesitter - After all of our initial testing we copy the queries that they have defined - If they differ, we run our highlight tests against them - We also issue a warning that the highlights have diverged (more than likely this will fail the highlighting tests anyways) This is in ref to tree-sitter#136
ckipp01
force-pushed
the
nvim-treesitter
branch
from
6b666a2 to
69c4f8e
Compare
| run: | | ||
| echo "::warning file=queries/highlights.scm::Queries in this repo are out of sync with nvim-treesitter" | ||
| git diff queries/highlights.scm | ||
| npm run test |
There was a problem hiding this comment.
Pretty sure the answer is "yes", but for discussion sake: should we necessarily fail the job if the queries are out of date?
PRs often change tree structure, and sometimes rename nodes - which breaks queries.
My personal opinion is that we are at a point (unless there's some catastrophic parser mistake current grammar has) where we can stabilise and treat breaking changes with a lot more care.
There was a problem hiding this comment.
My personal opinion is that we are at a point (unless there's some catastrophic parser mistake current grammar has) where we can stabilise and treat breaking changes with a lot more care.
I agree, except I'd say stabilize. If a PR breaks existing queries we should try working around it by creating a dummy node, or at least know that we are in that territory.
3 participants
This PR starts testing against the highlights that nvim-treesitter has
defined to ensure we don't break things without warning. The general
flow is as follows:
defined
likely this will fail the highlighting tests anyways)
This is in ref to #136. @keynmol let me know if this is what you had in mind.