report eastwood detail urls #124
Conversation
# Conflicts: # src/formatting_stack/linters/eastwood.clj # test/functional/formatting_stack/linters/eastwood.clj
…ack into eastwood-detail-urls # Conflicts: # test/functional/formatting_stack/linters/eastwood.clj
| {:source :eastwood/def-in-def | ||
| :line 3 | ||
| :column 13 | ||
| :warning-details-url "https://github.com/jonase/eastwood#def-in-def" |
There was a problem hiding this comment.
I find it interesting that match? allows the other test cases within this are to omit :warning-details-url and still pass.
I get why that's like that, but it also leads to:
- test case heterogeneity
- having different shapes of data is confusing - it makes me wonder "why the differences? do they have some intent relevant to the thing being tested?"
- currently only a fraction of the tests in this
areexercise the:warning-details-urlaspect- that literally means we get less coverage
- it also means that fewer tests act as documentation, which is an important role
- fragility
- I can delete random key-value entries in these maps and have the tests still pass
- The same cannot be said of most testing techniques/frameworks I know
- I can delete random key-value entries in these maps and have the tests still pass
As a tentative conclusion, match? is too DRY, up to the point of self-reference.
IMO a good test compares x against y. If there's only an x and no y, then x becomes a self-referential test (input == expectation).
Of course, #124 is not critical functionality, but as you know I've always been a bit skeptical about match? as a general-purpose tool. Maybe we can begin limiting its usage elsewhere.
There was a problem hiding this comment.
I find it interesting that
match?allows the other test cases within thisareto omit:warning-details-urland still pass.
The effect you're describing is only one of the matchers (matchers/embeds, default for maps). This behavior is fully configurable.
The default is set to embeds because maps are open to extension exact matches,. Breakage tend to be cumbersome and overwhelming when every are-case fails (e.g. because an extra key is added)
having different shapes of data is confusing
I'd say it might be unfamiliar. I've come to expect these shapes for the big-are clauses which leverage match?
currently only a fraction of the tests in this are exercise the
:warning-details-urlaspec
- that literally means we get less coverage
It means we get less duplicated coverage, which leads to more specific failures. one are-case failing helps pin-pointing the issue.
I can delete random key-value entries in these maps and have the tests still pass
From the expected-values, yes. the actual values, no.
The same cannot be said of most testing techniques/frameworks I know
It's rather common in Ruby's rspec (have-attributes-matcher, contains_exactly (similar to m/equals), and include), Java's hamcrest (IsIterableContaining, anyOf, arrayContaining, see matchers), python as a hamcrest impl as well.
Brief
the key is a string. conforming it to a keyword.
Now it'l show up in our reports :)
QA plan
green tests
Author checklist
Reviewer checklist