-
Notifications
You must be signed in to change notification settings - Fork 8
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
renovate: add test that should fail #137
base: main
Are you sure you want to change the base?
Conversation
@alvarocabanas If you could take a look at this it would be awesome ❤️ |
d290847
to
9559dbd
Compare
9559dbd
to
822600c
Compare
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.
LGTM
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 looks awesome @alvarocabanas, thanks a lot!
Left a quick doubt and after this we can merge this :)
@@ -41,9 +41,8 @@ func (c *commitsGetterMock) Commits(_ string) ([]git.Commit, error) { | |||
return commits, nil | |||
} | |||
|
|||
//nolint:funlen | |||
//nolint:funlen,paralleltest,maintidx |
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.
Why are we getting rid of paralleltest
? If this is unavoidable I'd love to see a short explanation.
Meant as an example rather than the actual refactor.
Renovate tests are currently missing some cases that should making them fail.
Moreover, when they fail, the output is very unreadable.
For example, consider this change:
The output of the test would be gigantic, because all expectations are offset by one:
Gigantic code block
This makes very hard to add new tests to the suite, as if the test does not work for one commit, all the others will fail as well. I think we should consider refactoring this test suite so it looks more like Dependabot's, which does not have this problem.