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

Show number of successful tests, classification + better gave up message #30

Merged
merged 2 commits into from Nov 2, 2023
Merged

Show number of successful tests, classification + better gave up message #30

merged 2 commits into from Nov 2, 2023

Conversation

sol
Copy link
Member

@sol sol commented Oct 27, 2023

Before

success before
gave-up before

After

success after
gave-up after

@sol sol mentioned this pull request Nov 1, 2023
@sol
Copy link
Member Author

sol commented Nov 1, 2023

Fixes #12.

@sol
Copy link
Member Author

sol commented Nov 1, 2023

@parsonsmatt any chance to get this merged?

Copy link
Collaborator

@parsonsmatt parsonsmatt left a comment

Choose a reason for hiding this comment

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

Overall looks good!

Do you mind adding a changelog entry?

Also curious on why a major version bump for this. No API change, no breaking behavior, just better error messages. I'd call that a patch change personally.

hspec-hedgehog.cabal Outdated Show resolved Hide resolved
@sol
Copy link
Member Author

sol commented Nov 1, 2023

Do you mind adding a changelog entry?

Done.

I'd call that a patch change personally.

Please go ahead with whatever you are most comfortable with.

Just for the future + to better understand where you are coming from:

In general, do you use versions to only communicate Haskell API changes, not behavior?

Or is it that you think in this specific situation the behavior changes are not significant, and that's why you ignore them in regards to versioning?

@parsonsmatt
Copy link
Collaborator

I consider the API change to be the "floor" for a version bump. Behavior changes can be a major/minor version, even if they don't alter the API, though, depending on a lot of factors.

To walk through my reasoning: The API doesn't change at all, so the smallest bump is patch. The behavior changes, but - it's human readable output, not intended for machine consumption. The output is strictly improved, IMO, so I guess that we won't get folks filing issues asking for the old behavior back. And, actual bugs are fixed. So we've got a lot of benefits to this patch. A smaller version bump gets shipped faster to folks and requires less OSS effort to integrate, so it's less effort involved.

So, how can this go wrong for folks, where we'd want to signal a minor or major bump? Well, IME, major upper version bounds tend to be a matter of taste. People that use them a lot are often not pinning dependencies (ie nix or stack or similar), or are writing open source libraries.

If the major upper bound has been placed on hspec-hedgehog by an OSS library, then anyone that wants to depend on hspec-hedgehog-0.2.0.0 needs to jailbreak or patch that library, just to get improved output. That's kind of annoying to end users, and does not impact the OSS library at all. If the OSS library is depending on API-level details, then it would have a minor version bump - so a major version bump is unnecessary. And if they're depending on stuff like the text output from the library in failure cases, then they really ought to have a patch level version bound already!

If the major upper bound on hspec-hedgehog is from an application, then they won't get bugfix+improved output until they opt in anyway. Do applications depend on the specific text output from a test suite? I would personally consider that pretty bad practice! But I grant it's possible. But then, they ought to be depending on a very tight version - perhaps, probably, even a pinned, specific version. So I think I'd argue that, in this case, sloppy version management and specification would be at fault. If we're going to cater to that sort of thing, then all changes would be major breaking changes, and there's no point to having minor or patch bumps at all.

So, for folks depending on hspec-hedgehog < 0.2.0.0, I guess that the vast majority of them want to upgrade, and it won't break their system. This makes me think a major bump wouldn't be great.

What about a minor bump? Depending on hspec-hedgehog < 0.1.1.0 signals that you are depending on hspec-hedghehog for the specific API interface, and are sensitive to additions to the API. Most of the same considerations apply above. Since we're not adding anything to the API, everything will continue to build - they'll just see different/improved test suite output. The costs of a minor version bump are smaller than the costs of a major version bump, so I can see that being appropriate.

ChangeLog.md Outdated Show resolved Hide resolved
hspec-hedgehog.cabal Outdated Show resolved Hide resolved
@parsonsmatt parsonsmatt merged commit af9178f into hspec:master Nov 2, 2023
5 checks passed
@parsonsmatt
Copy link
Collaborator

Released as hspec-hedgehog-0.1.1.0

@sol
Copy link
Member Author

sol commented Nov 3, 2023

@parsonsmatt I think that's a consistent school of thought. On some points I have a different perspective though, which I think is fine. We don't have to reconcile this. I also think that discussing this extensively could get, well, quite extensive, I guess. So let's not aim for that.

I left a couple of points to emphasize my perspective. If you have any input that you think fosters an insightful discussion, then please leave replies, but by no means feel obligated to. Again, I think this whole topic is far too big for a GitHub discussion.

I consider the API change to be the "floor" for a version bump. Behavior changes can be a major/minor version, even if they don't alter the API, though, depending on a lot of factors.

That makes a lot of sense.

To walk through my reasoning: The API doesn't change at all, so the smallest bump is patch. The behavior changes, but - it's human readable output, not intended for machine consumption. The output is strictly improved, IMO, so I guess that we won't get folks filing issues asking for the old behavior back. And, actual bugs are fixed. So we've got a lot of benefits to this patch. A smaller version bump gets shipped faster to folks and requires less OSS effort to integrate, so it's less effort involved.

  • Personally, I try to only ever do patch level releases when I'm close to 100% certain that I don't introduce new bugs. That means that when I do a patch level release then a consumer can assume that the new release is a strict improvement. Whenever you write any meaningful amount of code, there is a chance that you introduce a new bug. So in which situation is it safe to assume that code changes are strict improvements? When I have a program that crashes and I apply a careful fix that turns the crash condition into some more useful behavior, that's a strict improvement. Depending on what code I had to touch I may still be close to 100% certain that I didn't introduce any new bugs. That's pretty rare, and that's why you don't see a lot of patch level releases from me.
  • Another take could be: For a new patch level release x.y.z.p1 you should be able to use x.y.z.p1 as a functionally equivalent substitute for any x.y.z.p0, where p0 < p1. That means, existing defined behavior should not change in patch level releases, only where the existing behavior was undefined (crashed / diverged) changes are permissible.
  • If larger version bumps are expensive, and for that reason I am tempted to "smuggle" things into patch level releases, then I think that's a problem. A problem that I would want to solve by somehow making larger version bumps less expensive.

Well, IME, major upper version bounds tend to be a matter of taste. People that use them a lot are often not pinning dependencies (ie nix or stack or similar)

  • I'm skeptical whether this holds much weight. Not using version pinning is antiquated and that cabal does not create freeze files by default is pretty fringe. People who do not use version pinning for production applications do not really know what they are doing! By extension, I could read that statement as "people that do major version bumps a lot don't really know what they are doing", and I'm not sure if I would agree with that statement.

If the major upper bound has been placed on hspec-hedgehog by an OSS library, then anyone that wants to depend on hspec-hedgehog-0.2.0.0 needs to jailbreak or patch that library, just to get improved output. That's kind of annoying to end users, and does not impact the OSS library at all. If the OSS library is depending on API-level details, then it would have a minor version bump - so a major version bump is unnecessary.

  • Without going into much depth, at leas from my perspective, upper bounds don't pull their own weight. This "bump upper bounds"-chores are pretty binary decisions: Does CI pass (which most of the time it does)? If yes, then Hackage revision + PR that bumps the upper bound.
  • It's far less work to omit upper bounds and do Hackage revisions after the fact. It's not perfect and everybody has to decide themselves where they are willing to compromise, but I advise to do it in a way that does not promote RSI.
  • Consumers don't appreciate breaking API changes in general, major version bump, or not + I have seen breaking changes in minor versions more than once (manual versioning is notoriously unreliable). Upper bounds don't guarantee that what's on Hackage does actually build. They may make build failures less likely, but they also come at a huge cost. I'm not advocating for never using upper bounds. I, personally, decide this on a case by case basis, individually for every dependency. How stable is the library? How deep do I reach into internals of the library? How fast can I fix things after the fact?

And if they're depending on stuff like the text output from the library in failure cases, then they really ought to have a patch level version bound already!

  • For hspec-hedgehog specifically, it's really not that important what you do here. But for example doctest does depend on GHC output, and I don't anticipate that this output changes between minor releases.

If the major upper bound on hspec-hedgehog is from an application, then they won't get bugfix+improved output until they opt in anyway. Do applications depend on the specific text output from a test suite? I would personally consider that pretty bad practice! But I grant it's possible. But then, they ought to be depending on a very tight version - perhaps, probably, even a pinned, specific version. So I think I'd argue that, in this case, sloppy version management and specification would be at fault.

  • I mean yes, again, we agree here in so far, that not doing version pinning is pretty fringe.
  • But: At the point you pin your dependency versions, does it still matter much whether you get a major, minor, or patch bump from a dependency? Version pinning acknowledges the fact that manual versioning is notoriously unreliable, and that humans make mistakes. For that reason you freeze an exact set of transitive dependency versions. When you update your dependencies you freeze a new set of versions. You never deal with version ranges, and in the absence of version ranges, I would argue, that versions lose their semantics. All you care about is that a specific version works for you. The literal version number itself serves as kind of a unique identifier to address that thing that works for you.
  • I acknowledge though that that last point may not be true for stack. Stackage treats patch level releases differently (afaik).

So, for folks depending on hspec-hedgehog < 0.2.0.0, I guess that the vast majority of them want to upgrade, and it won't break their system. This makes me think a major bump wouldn't be great.

  • Again, they shouldn't. Upper bounds are a scam. What they should do is pin their dependency versions + update their pinned dependencies regularly.

What about a minor bump?

  • That works great for me.

The costs of a minor version bump are smaller than the costs of a major version bump, so I can see that being appropriate.

  • Again, I think that's the actual problem.

@sol sol deleted the show-clasifications branch November 3, 2023 08:46
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

2 participants