-
Notifications
You must be signed in to change notification settings - Fork 2
Add CI Linting #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CI Linting #22
Conversation
Interesting - the general consensus has been to move away from Plenary for testing and move towards a simpler |
.github/workflows/run-tests.yml
Outdated
|
||
name: Run tests | ||
|
||
# on: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the commented settings here tell the CI to run on every pull request, but only on pushes to main. In order to test this, I had to make it run on every push/pull request. This is probably not economical, but I can confirm that It works and the test passes.
.github/workflows/run-tests.yml
Outdated
steps: | ||
- uses: actions/checkout@v3 | ||
- name: Run tests | ||
uses: nvim-neorocks/nvim-busted-action@v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This action does a pretty good job of setting up everything and running the specs. If you look at the output, it's doing a whole lot (which is necessary) - perhaps there is a way to containerize or mimic this action locally so that the tests are easier to run in the local development environment without contending with a bunch of properly configured dependencies.
I appreciate the effort you put into this, but I am not sure I want to introduce testing. I wouldn't mind a CI check to ensure the files are linted, but I can't personally see myself adding tests to this project as my time is already in such short supply. Testing a plugin of an application which loads another application feels like a lift I'm not prepared for at this point. But if you want to refactor to call stylua and then abort if it doesn't lint correctly, we could probably accept that. LMK. |
Totally makes sense. This was really just a way for me to try and learn about testing in Lua, which I did! Let me see how a Stylua lint might look. I think that's a good compromise. |
This is a proof-of-concept with a single test (that will always pass). It runs on ubuntu and mac and with a couple different versions of Neovim. I'm not quite sure yet if my proposed directory structure is the best way forward, but this should at least give us a good starting point to run tests on push.We might also want to talk about when these tests should run - every push? on push to MR that merges into main? I don't know much about CI or what the conventions are, but I can think about it a bit more.I've reworked this MR to do a simple Stylua check on the code, since testing seems to be outside the scope of this plugin. I've tested locally that it will fail when Stylua fails, and pass when Stylua passes.