Skip to content

haskellPackages: re-enable tests for many packages#384127

Merged
maralorn merged 2 commits intoNixOS:haskell-updatesfrom
alexfmpe:sweep-dontCheck
Mar 9, 2025
Merged

haskellPackages: re-enable tests for many packages#384127
maralorn merged 2 commits intoNixOS:haskell-updatesfrom
alexfmpe:sweep-dontCheck

Conversation

@alexfmpe
Copy link
Member

I've been prototyping a snippet that returns all derivations for which doCheck = false, but with the flag set to true instead. Then I build the whole bunch against haskellPackages and see which ones succeed. Out of ~300 ones, close to 100 built.

I didn't remove a good chunk of them that had scary comments, e.g.

  • 32 bit only behavior (not sure how we want to test that)
  • mutual cycles between test packages
  • non determinism in tests

These ones also built but they have doCheck = false right in hackage-packages.nix. Not sure why

  • Cabal-3.10
  • Cabal-3.12
  • Cabal-3.14
  • haskell-src-exts
  • mwc-random
  • req
  • websockets

Before and after this PR there's no failures when running

nix-build -E 'with (import ./. {}).haskell.packages.ghc98; [ HList Random123 configuration-tools css-text data-hash direct-sqlite dlist docopt doctest-parallel domain-auth fft fsnotify ghcide haeredes hath hedn hindent hspec-contrib implicit isocline memcache metrics mfsolve monad-dijkstra mysql-haskell netpbm network network-transport-tcp path-io pipes-websockets raven-haskell safecopy sequence-formats servant-auth-client socket store snap-core strive system-fileio system-filepath systemd th-printf token-bucket tree-diff typed-process unagi-chan unicode-data unordered-containers xml-picklers wai-logger webdriver wuss yi-keymap-vim zip-archive ]'

and there are exactly these failures when using ghc910: HList, hindent ghcide

Given the large volume, I wouldn't be surprised if we eventually end up having to add a few back, but hard to tell the reasons for a doCheck when there's no comment.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: haskell General-purpose, statically typed, purely functional programming language label Feb 22, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 2501-5000 This PR causes many rebuilds on Darwin and should target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. labels Feb 22, 2025
@alexfmpe
Copy link
Member Author

Note I have only tested on x86_64-linux so far.
Even GHC 9.8 is apparently not cached yet for aarch64-darwin on haskell-updates.

Copy link
Member

@maralorn maralorn left a comment

Choose a reason for hiding this comment

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

I fear that some of them are flaky. But I still think we should bite this bullet.

@alexfmpe
Copy link
Member Author

alexfmpe commented Feb 24, 2025

I fear that some of them are flaky. But I still think we should bite this bullet.

My thoughts exactly. I kept a ton that "built on my machine" because their comments said they were flaky, but for the ones that offer no reason, I'd err on the side of taking them off and possibly adding back when they bite us with an explanation rather than keep them until some eventual manual fine-grained sweep.

Maybe we ought to even require a reason added (e.g. issue/pull link) for new overrides so they don't stick around forever. Not too long ago, configuration-common was just over 3k LOC, it's 10% less at this point

@alexfmpe alexfmpe force-pushed the sweep-dontCheck branch 2 times, most recently from ad5a9eb to e618571 Compare March 2, 2025 14:11
@maralorn maralorn merged commit 4a6f7e5 into NixOS:haskell-updates Mar 9, 2025
27 checks passed
@maralorn
Copy link
Member

maralorn commented Mar 9, 2025

Love it.

@alexfmpe alexfmpe deleted the sweep-dontCheck branch June 14, 2025 19:55
@vcunat
Copy link
Member

vcunat commented Jun 23, 2025

So, I was researching why .tree-diff failed on Hydra. Why did you reenable tests for it? It linked to upstream issue that hasn't been resolved, so why are we doing this dance again?

@vcunat
Copy link
Member

vcunat commented Jun 23, 2025

I mean, if even upstream doesn't care for years to fix a test failure, will we do something about it ourselves? (except keep restarting the builds manually) When I enable tests, I think about what we do in case they fail, because otherwise they're more harm than good.

@maralorn
Copy link
Member

I guess the why is that the test passed and alex probably overlooked the fact that the issue stated it was flaky. Let’s disable them.

@sternenseemann
Copy link
Member

I think I'll just disable this test case (specifically) again. Looks like a bug in the exprParser w.r.t. whitespace handling. The parser doesn't seem that important in the first place.

Also, it seems to me that this dontCheck shouldn't have been removed in the first place with the reasoning given in this PR (removing those overrides that give no explanation/upstream issue). I've also noticed in practice that it is difficult making the connection to this change again when running into these types of failures, so I more often than not just restart them instead of remembering that I should probably re-insert a dontCheck

sternenseemann added a commit to sternenseemann/nixpkgs that referenced this pull request Jun 23, 2025
There hasn't been movement on this in many years since the package seems
to be in maintenance mode only, but it is a prominently used dependency,
so we'll have to ignore this, pretty much.

Reference haskellari/tree-diff#79.
Reference NixOS#384127 (comment).
nixpkgs-ci bot pushed a commit that referenced this pull request Jun 29, 2025
There hasn't been movement on this in many years since the package seems
to be in maintenance mode only, but it is a prominently used dependency,
so we'll have to ignore this, pretty much.

Reference haskellari/tree-diff#79.
Reference #384127 (comment).

(cherry picked from commit 50b7532)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 2501-5000 This PR causes many rebuilds on Darwin and should target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants