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

Adding More Tests for Commands & Movement #144

Closed
kirawi opened this issue Jun 6, 2021 · 7 comments
Closed

Adding More Tests for Commands & Movement #144

kirawi opened this issue Jun 6, 2021 · 7 comments
Labels
C-enhancement Category: Improvements E-easy Call for participation: Experience needed to fix: Easy / not much

Comments

@kirawi
Copy link
Member

kirawi commented Jun 6, 2021

I think it would be beneficial to thoroughly test helix-core/src/movement.rs & helix-term/src/commands.rs. To start, it would probably be most important to test the functions transitively called by hotkeys in helix-term/src/keymap.rs. This could also be a good first issue.

@hovsater
Copy link
Contributor

hovsater commented Jun 6, 2021

I agree. Not only would it be nice to have it tested, it would serve us well in terms of catching regressions. 🙂

What kind of tests are you envisioning? While unit tests are nice, I think integration tests would be ideal here.

I remember mle having pretty sweet integration tests as an example.

@kirawi
Copy link
Member Author

kirawi commented Jun 6, 2021

I was thinking of having basic unit tests to check for correct behaviour and that it doesn't panic, and then later on have integration tests to address #115 and basically include complicated cases, combos, etc. However, I think this and that should be considered separate issues. Feel free to disagree, I don't have much experience in this area so I'll defer to you.

@pickfire
Copy link
Contributor

pickfire commented Jun 7, 2021

kakoune have nice integration tests, we could do some macros for these.

@PabloMansanet
Copy link
Contributor

I'm happy to write a few of these, as it tends to be what I do when I'm wrapping my head around a codebase anyway :). Feel free to assign me if nobody else is on this.

@kirawi
Copy link
Member Author

kirawi commented Jun 12, 2021

Can this be considered completed or do we want to improve our test coverage further in this specific portion of Helix?

@archseer
Copy link
Member

We extensively covered movement but I'd still like to cover other commands.

@CBenoit CBenoit added the C-enhancement Category: Improvements label Jun 18, 2021
@kirawi kirawi added the E-easy Call for participation: Experience needed to fix: Easy / not much label Aug 19, 2021
@archseer
Copy link
Member

This is covered by #115

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Improvements E-easy Call for participation: Experience needed to fix: Easy / not much
Projects
None yet
Development

No branches or pull requests

6 participants