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

util: Test #781

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Conversation

ahmad-abuziad
Copy link

@ahmad-abuziad ahmad-abuziad commented Jan 22, 2025

The goal of this PR to test cover util/util.go and increase it from 64.8% to 100% since almost all the util functions are testable
Screenshot From 2025-01-22 13-43-38

@purpleidea
Copy link
Owner

These will keep failing because of the commit message. Look at the tests. Lmk if you need more help.

@ahmad-abuziad
Copy link
Author

These will keep failing because of the commit message. Look at the tests. Lmk if you need more help.

I will look into it.
I will stick with the format since i started with it, except the last commit will contain a message that doesn't fail.

@purpleidea
Copy link
Owner

purpleidea commented Jan 26, 2025

You can easily rename a bunch with: EDITOR=vim git rebase -i <commit id before the first one>

@ahmad-abuziad
Copy link
Author

coverage increased from 64.8% to 94.4
image

@ahmad-abuziad ahmad-abuziad marked this pull request as ready for review January 30, 2025 05:51
Copy link
Owner

@purpleidea purpleidea left a comment

Choose a reason for hiding this comment

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

Thanks for all the tests.

For a first start, can you please rename the commit messages so they have the right format, typically util: Test blah blah... in this case.

Next see the other misc things, then we'll do a final review. Thank you!

Sorry for the delay, I've been at the FOSDEM+CfgMgmtCamp conference stuff.

@ahmad-abuziad ahmad-abuziad force-pushed the util-util-test branch 2 times, most recently from 3858b69 to c3af7a4 Compare February 10, 2025 08:34
@ahmad-abuziad ahmad-abuziad changed the title Test: util/util.go util: test Feb 10, 2025
@ahmad-abuziad ahmad-abuziad changed the title util: test util: test util file Feb 10, 2025
@ahmad-abuziad ahmad-abuziad changed the title util: test util file util: test Feb 10, 2025
@purpleidea
Copy link
Owner

I expect to still see failures, test is here: https://github.com/purpleidea/mgmt/blob/master/test/test-commit-message.sh#L17

So for example a valid commit is:

foo: Hello something

an invalid commit is:

foo: hello something

Note the capitalization.

@ahmad-abuziad ahmad-abuziad changed the title util: test util: Test Feb 11, 2025
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