Remove bazel-diff and bazel-differ comparison tests#106
Remove bazel-diff and bazel-differ comparison tests#106shs96c merged 1 commit intobazel-contrib:mainfrom
Conversation
These were useful while we were bootstrapping the project, but we are not updating these any more, and we are unlikely to change the behaviour of Target Determinator if an update causes some of these comparison tests to fail. For both of these reasons, it makes sense to remove the tests.
sitaktif
left a comment
sitaktif
left a comment
There was a problem hiding this comment.
I'd argue these tests are still useful to maintain the project and compare featuresets.
bazel-differ seems to gather a bit of dust but bazel-diff is still a good tool to compare against.
|
We (@sitaktif , @shs96c , and I) discussed this, and agreed that it does make sense to merge this change. This test suite was initially very valuable, but given where we are now, it adds overhead and complexity to our tests, and we don't expect future contributors to dive into differences between the two tools and update the test suite to clarify what the difference is. Recent PRs that hit differences between the tools' output have just ignored the failing tests, which lends weight to that assumption. |
shs96c
dismissed
sitaktif’s stale review
It is useful to know what the differences are between the various tools, but I don't think we're giving proper justice by not actively tracking the others and reviewing our own tests.
|
I really like the documentation provided by the tests, but the reality is that most of these overridden tests are just We can re-add these tests with specific assertions in the future if the time investment provides enough value. |
3 participants
These were useful while we were bootstrapping the project, but we are not updating these any more, and we are unlikely to change the behaviour of Target Determinator if an update causes some of these comparison tests to fail.
For both of these reasons, it makes sense to remove the tests.