Skip to content

Delete watchdogProp function #1227

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

newhoggy
Copy link
Contributor

@newhoggy newhoggy commented Jun 25, 2025

Changelog

- description: |
    Delete `watchdogProp` function
# uncomment types applicable to the change:
  type:
  # - feature        # introduces a new feature
  # - breaking       # the API has changed in a breaking way
  # - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - refactoring    # QoL changes
  # - bugfix         # fixes a defect
  - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

watchdogProp can be deleted because we are now using a new version of hedgehog-extras that fixes a hanging bug in moduleWorkspace. After fixing this bug, tests do not hang anymore so we do not need watchdogProp to interrupt the hanging test.

The PR that fixed moduleWorkspace is here: input-output-hk/hedgehog-extras#91

The reason the old version of moduleWorkspace hangs is not fully known, however it uses a combination of ResourceT and recovering retryPolicy, which causes the hang.

Aside from the fact that this combination hangs, it is problematic to use ResourceT when the intention is to cleanup resources in a timely manner. The scope of ResourceT is runResourceT which is outside the call to moduleWorkspace. This means that when moduleWorkspace returns, there is no guarantee that the module directory is cleaned up as it is supposed to.

In order to ensure timely cleanup of resources, the bracket family of functions should be used instead.

How to trust this PR

Highlight important bits of the PR that will make the review faster. If there are commands the reviewer can run to observe the new behavior, describe them.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff

@newhoggy newhoggy marked this pull request as ready for review June 25, 2025 13:00
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Can you add an explanation in the description as to why we can delete watchdogProp?

@newhoggy newhoggy requested a review from Jimbo4350 June 26, 2025 13:58
@newhoggy newhoggy enabled auto-merge June 26, 2025 14:13
@carbolymer carbolymer disabled auto-merge June 26, 2025 18:02
Copy link
Contributor

@carbolymer carbolymer left a comment

Choose a reason for hiding this comment

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

@newhoggy newhoggy force-pushed the newhoggy/delete-watchdogProp-function branch from 9b0f65e to 0ac4d44 Compare June 27, 2025 09:47
@newhoggy newhoggy requested review from a team as code owners June 27, 2025 09:47
@newhoggy newhoggy force-pushed the newhoggy/delete-watchdogProp-function branch from 0ac4d44 to 0ea99e2 Compare June 28, 2025 01:00
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.

3 participants