Skip to content

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

Closed
wants to merge 11 commits into from
Closed

Add CI Linting #22

wants to merge 11 commits into from

Conversation

body-clock
Copy link
Contributor

@body-clock body-clock commented Apr 18, 2025

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.

@body-clock
Copy link
Contributor Author

Interesting - the general consensus has been to move away from Plenary for testing and move towards a simpler busted setup. From what I understand, Plenary seemed to be the go-to because you couldn't run nvim as a lua interpreter. Now, with nvim -l that's actually possible - leading the move towards raw busted testing with a rockspec and .busted file. More info in the nvim-lua-plugin-template from nvim-lua.

@body-clock body-clock changed the title Add CI testing with PlenaryBustedDirectory Add CI Testing Apr 21, 2025
@body-clock body-clock marked this pull request as ready for review April 21, 2025 18:42

name: Run tests

# on:
Copy link
Contributor Author

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.

steps:
- uses: actions/checkout@v3
- name: Run tests
uses: nvim-neorocks/nvim-busted-action@v1
Copy link
Contributor Author

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.

@adam12
Copy link
Owner

adam12 commented Apr 26, 2025

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.

@body-clock
Copy link
Contributor Author

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.

@body-clock body-clock changed the title Add CI Testing Add CI Linting Apr 28, 2025
@body-clock body-clock closed this Apr 28, 2025
@body-clock body-clock deleted the busted-ci branch April 28, 2025 14:49
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.

2 participants