search

Antonboom / testifylint

The Golang linter that checks usage of github.com/stretchr/testify.
https://github.com/stretchr/testify
MIT License
101 stars 8 forks source link

new checker `contains` #152

Closed mmorel-35 closed 5 months ago

mmorel-35 commented 5 months ago

Closes #54

coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9632081552

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 37 47 78.72%
<!-- Total: 40 50 80.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.2%
Covered Lines: 2215
Relevant Lines: 2370
💛 - Coveralls
mmorel-35 commented 5 months ago

I believe https://github.com/Antonboom/testifylint/issues/47

could be a subcase of contains but I'm not so confident with providing it in a error-compare rule

ccoVeille commented 5 months ago

I beli if https://github.com/Antonboom/testifylint/issues/47

Shall be a subcase of contains or in a error-compare rule

Because of the strange use cases I reported here, I'm unsure.

https://github.com/Antonboom/testifylint/issues/47#issuecomment-2047862514github.com/Antonboom/testifylint/issues/47#issuecomment-2047862514

But they are definitely close yes

coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9635529714

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 35 47 74.47%
<!-- Total: 38 50 76.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.3%
Covered Lines: 2213
Relevant Lines: 2370
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9635616469

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 35 47 74.47%
<!-- Total: 38 50 76.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.3%
Covered Lines: 2213
Relevant Lines: 2370
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9635658244

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 39 47 82.98%
<!-- Total: 42 50 84.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.1%
Covered Lines: 2217
Relevant Lines: 2370
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9635733489

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 39 47 82.98%
<!-- Total: 42 50 84.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.1%
Covered Lines: 2217
Relevant Lines: 2370
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9636072731

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 39 47 82.98%
<!-- Total: 42 50 84.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.1%
Covered Lines: 2217
Relevant Lines: 2370
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9636090503

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 39 47 82.98%
<!-- Total: 42 50 84.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.1%
Covered Lines: 2217
Relevant Lines: 2370
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9639678060

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 39 47 82.98%
<!-- Total: 42 50 84.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.1%
Covered Lines: 2217
Relevant Lines: 2370
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9639712727

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 39 47 82.98%
<!-- Total: 42 50 84.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.1%
Covered Lines: 2217
Relevant Lines: 2370
💛 - Coveralls
ccoVeille commented 5 months ago

LGTM 👍, a few stylistic remarks

coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9639996723

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 39 47 82.98%
<!-- Total: 42 50 84.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.1%
Covered Lines: 2217
Relevant Lines: 2370
💛 - Coveralls
ccoVeille commented 5 months ago

You did the changes but didn't regenerate and commit the test and golden files

mmorel-35 commented 5 months ago

I did from my phone, so I have no access to a terminal. The CI shall be doing the warning

ccoVeille commented 5 months ago

I did from my phone, so I have no access to a terminal. The CI shall be doing the warning

yes, I agree it should. Something to be added, because it's a common mistake, I did it too.

I don't see something that could catch it right now https://github.com/Antonboom/testifylint/blob/master/.github/workflows/ci.yml

what are your thoughts @Antonboom ?

coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9656918190

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 39 47 82.98%
<!-- Total: 42 50 84.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.1%
Covered Lines: 2217
Relevant Lines: 2370
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9656928741

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 39 47 82.98%
<!-- Total: 42 50 84.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.1%
Covered Lines: 2217
Relevant Lines: 2370
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9656934424

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 39 47 82.98%
<!-- Total: 42 50 84.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.1%
Covered Lines: 2217
Relevant Lines: 2370
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9656951923

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 39 47 82.98%
<!-- Total: 42 50 84.0% -->
Totals Coverage Status
Change from base Build 9608177733: -0.1%
Covered Lines: 2217
Relevant Lines: 2370
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9666920543

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/checkers/contains.go 35 39 89.74%
<!-- Total: 38 42 90.48% -->
Totals Coverage Status
Change from base Build 9608177733: 0.03%
Covered Lines: 2213
Relevant Lines: 2362
💛 - Coveralls
ccoVeille commented 5 months ago

Cannot run on my codebase for the moment, but LGTM 👍

Antonboom commented 5 months ago

I don't get where is the magic?

because of testdata directory

ccoVeille commented 5 months ago

Thanks I learned something today.

I had no idea the testdata folder I had on my codebase was not a convention in our codebase but something idiomatic