Skip to content
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

Automated testing with Luacheck #533

Merged
merged 1 commit into from
Jun 11, 2022
Merged

Automated testing with Luacheck #533

merged 1 commit into from
Jun 11, 2022

Conversation

thomascft
Copy link
Contributor

This is my fix for #527.
Well, I don't know git well enough clearly. I setup the testing, you can see the output here. I had a bit of cleanup, so that's what the force pushes were

@thomascft
Copy link
Contributor Author

I should probably mention that the test rules are defined in .luacheckrc with the documentation over here.

@lcpz lcpz merged commit aa2f913 into lcpz:master Jun 11, 2022
@lcpz
Copy link
Owner

lcpz commented Jun 11, 2022

Thank you for this. However, we still need unit tests.

It's not so difficult now, we just need to create a test script with luaunit and run it in the Action you created, just after the linter call.

Feel free to continue to help me, I think it's a nice exercise for you.

Otherwise, I will work on it as soon as I can.

name: Luacheck
on: [push]
jobs:
Luacheck_Tests:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the next PR, you can add another job, say unit_tests, with a step in which you run the testing script.

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'll try it out, should be able to get it in by Wednesday.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you set on luaunit? Busted seems to have much better documentation, and the AwesomeWM team said it's what they use in #527. It would be nice to see how they've structured unit testing.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Busted is fine. Take inspiration from AwesomeWM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly do you need tested? I haven't done unit testing before and I'm not sure it's being done correctly.

Copy link
Owner

@lcpz lcpz Jun 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to launch an Awesome instance in which we load all the widgets (with all possible configurations) to see if they work. This test suite would be called to verify that each commit does not break anything.

I know this is a lot of work, but you can start by launching Awesome as mentioned here. Then, we can incrementally add tests.

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