Skip to content

tests: make UserConfig unit tests concurrency-safe with unique tempiles#11759

Open
ulysses4ever wants to merge 6 commits intohaskell:masterfrom
ulysses4ever:master
Open

tests: make UserConfig unit tests concurrency-safe with unique tempiles#11759
ulysses4ever wants to merge 6 commits intohaskell:masterfrom
ulysses4ever:master

Conversation

@ulysses4ever
Copy link
Copy Markdown
Collaborator

@ulysses4ever ulysses4ever commented Apr 20, 2026

Fix #11686. Created with Copilot.


Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

@Bodigrim
Copy link
Copy Markdown
Collaborator

It adds some more cleanup than strictly what #11686 asks for (the teardown function removes some extra *.tmp files) but it looks legit: it addresses this logic in Distribution.Client.Config

Rather than work around it in the test suite, let's fix Distribution.Client.Config: the shenanigans with tmpFile are ill advised, just createDirectoryIfMissing and immediately writeFileAtomic file.

@geekosaur
Copy link
Copy Markdown
Collaborator

FWIW I agree with Bodigrim.

@ulysses4ever
Copy link
Copy Markdown
Collaborator Author

@Bodigrim done. There's also a new test that tries to concurrently create user configs. Let me know if it doesn't make sense, I can remove it.

@Bodigrim
Copy link
Copy Markdown
Collaborator

There's also a new test that tries to concurrently create user configs. Let me know if it doesn't make sense, I can remove it.

It does not make sense to me.

@ulysses4ever
Copy link
Copy Markdown
Collaborator Author

@Bodigrim very well, the test is removed. The only potentially slippery spot I can think of is the bytestring-management business that had to be added for writeFileAtomic (writeFileAtomic file $ LBS.fromStrict . toUTF8BS $ ...). If there's a beter way to get it done, let me know. Otherwise this should be ready hopefully.

@Bodigrim
Copy link
Copy Markdown
Collaborator

If there's a beter way to get it done, let me know.

One can construct StrictByteString directly (by replacing Disp.render in showConfigWithComments with Disp.fullRender), but this perhaps goes beyond the scope of the issue. I'm fine either way.

@philderbeast
Copy link
Copy Markdown
Collaborator

@ulysses4ever this seems to have fixed the problem with #11686. After a few runs, maybe the 4th, I got this error:

$ cabal test cabal-install:unit-tests
...
    getProjectRootUsability
      relative path:                                                                                                FAIL (0.02s)
        tests/UnitTests/Distribution/Client/ProjectConfig.hs:122:
        expected: ProjectRootUsabilityPresentAndUsable
         but got: ProjectRootUsabilityNotPresent
        Use -p '/relative path/' to rerun this test only.

@Bodigrim
Copy link
Copy Markdown
Collaborator

After a few runs, maybe the 4th, I got this error...

AFAICT this is not a regression caused by this PR, but rather another facet of upgrade to tasty-1.5.4: running testGetProjectRootUsability concurrently is expected to fail. I'd suggest to raise a separate ticket.

@ulysses4ever
Copy link
Copy Markdown
Collaborator Author

@Bodigrim

but this perhaps goes beyond the scope of the issue. I'm fine either way.

I'd then stick to what's there currently.

Copilot AI and others added 6 commits April 22, 2026 22:21
…iles (#26)

tests: use unique temp files in UserConfig bracketTest

Agent-Logs-Url: https://github.com/ulysses4ever/cabal/sessions/043e3696-3cda-43aa-aa50-e483ec531d13

Co-authored-by: ulysses4ever <6832600+ulysses4ever@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ulysses4ever <6832600+ulysses4ever@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ulysses4ever <6832600+ulysses4ever@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"resource busy (file is locked)" failures running tests locally

5 participants