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

Matchers: has.ElementsWith and has.StructWithValues #1

Merged
merged 12 commits into from
Dec 14, 2020

Conversation

jpolack
Copy link
Contributor

@jpolack jpolack commented Dec 8, 2020

I implemented two new matchers:

has.ElementsWith: Checks wether each element of an array/slice matches all expectations
has.StructWithValues: Allows to check single struct fields

Can be combined in a way that now we can check if f.e.:

[
  {
    "id": "someId1",
    "someOmittedField": true
  },
  {
    "id": "someId2",
    "someOmittedField": false
  }
]

has structs that have an id field with the prefix someId

@coveralls
Copy link

coveralls commented Dec 8, 2020

Coverage Status

Coverage decreased (-0.4%) to 99.58% when pulling 18016b7 on jpolack:master into 56a1405 on corbym:master.

Copy link
Owner

@corbym corbym 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 this, these matchers look good.

In addition to my comments, please take a look at the test coverage - coveralls seems to be complaining that there is some missing coverage somewhere.

Please address my concerns when you get a chance and I will merge your PR.

has/haselements.go Outdated Show resolved Hide resolved
has/hasstructvalues.go Outdated Show resolved Hide resolved
@jpolack
Copy link
Contributor Author

jpolack commented Dec 9, 2020

Thanks for your Feedback!

I am wondering why coveralls does not track that last line. I checked it locally and this test case should cover the missing line: https://github.com/corbym/gocrest/pull/1/files#diff-1a13e117c42f75f290b07b9587f7401c2f91a32980298f6e0bc547de98865e51R817 🤔

Is there a possiblity to call coveralls locally? go test ./... -cover just gives me the coverage for the gocrest package but not the subpackages.

@jpolack
Copy link
Contributor Author

jpolack commented Dec 9, 2020

One thing I want to get mentioned:
With these matchers one can write quite complex assertions. And the way we currently describe the errors might be too long, because atm we would print all assertions, even the ones that succeded.
A future improvement could be to change that and just print the assertions that failed (maybe with a property path) and the actual value (maybe as a stack)

@corbym
Copy link
Owner

corbym commented Dec 10, 2020

Thanks for your Feedback!

I am wondering why coveralls does not track that last line. I checked it locally and this test case should cover the missing line: https://github.com/corbym/gocrest/pull/1/files#diff-1a13e117c42f75f290b07b9587f7401c2f91a32980298f6e0bc547de98865e51R817 🤔

Is there a possiblity to call coveralls locally? go test ./... -cover just gives me the coverage for the gocrest package but not the subpackages.

I think coveralls is having a bit of a paddy. I will merge your code and then check out what is happening from there.

has/haselements.go Outdated Show resolved Hide resolved
has/haselements.go Outdated Show resolved Hide resolved
has/hasstructvalues.go Outdated Show resolved Hide resolved
Copy link
Owner

@corbym corbym left a comment

Choose a reason for hiding this comment

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

Looking good.. just one last thing I think!

has/haseveryelement.go Show resolved Hide resolved
@corbym corbym merged commit 2e248ec into corbym:master Dec 14, 2020
@corbym
Copy link
Owner

corbym commented Dec 15, 2020

One thing I want to get mentioned:
With these matchers one can write quite complex assertions. And the way we currently describe the errors might be too long, because atm we would print all assertions, even the ones that succeded.
A future improvement could be to change that and just print the assertions that failed (maybe with a property path) and the actual value (maybe as a stack)

I have opened an issue for this. It will be esp. useful when allOf and anyOf or the variadic parametered matchers are called.

@corbym
Copy link
Owner

corbym commented Dec 15, 2020

Thanks for your Feedback!

I am wondering why coveralls does not track that last line. I checked it locally and this test case should cover the missing line: https://github.com/corbym/gocrest/pull/1/files#diff-1a13e117c42f75f290b07b9587f7401c2f91a32980298f6e0bc547de98865e51R817 🤔

Is there a possiblity to call coveralls locally? go test ./... -cover just gives me the coverage for the gocrest package but not the subpackages.

I found the reason for this: the test was assigning the actual string to an interface{} type and that was Panicking in the for loop rather than the default method.

This has been fixed.

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.

None yet

3 participants