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

Change the test expectation md5 hash #9487

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

philderbeast
Copy link
Collaborator

@philderbeast philderbeast commented Dec 1, 2023

See #9486. Change the test expectation md5Hash for two tests.

I also removed MIN_VERSION_base(4,7,0) conditionals as we can't build anyway before ghc-8.4.

@philderbeast
Copy link
Collaborator Author

Why has the hash has changed?

@andreabedini
Copy link
Collaborator

Why has the hash has changed?

I guess if some types from base change their representation, it might get get reflected in our Structure description.

Copy link
Collaborator

@andreabedini andreabedini left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

If we were sure what's the cause of this discrepancy, it would be valuable to explain it in comments just before the ifdefs (e.g., if it comes from GHC vs from base, for the future, where one GHC can handle multiple base versions). But since we are not sure, this is good enough. Thank you.

@philderbeast
Copy link
Collaborator Author

I got a test failure in CI, with Validate macos-latest ghc-9.2.8 but it doesn't seem related. @Mikolaj can we prod just that one test to run again?

-----BEGIN CABAL OUTPUT-----
Error: cabal: '/usr/local/opt/curl/bin/curl' exited with an error:
curl: (7) Failed to connect to localhost port 8000 after 0 ms: Couldn't connect to server
-----END CABAL OUTPUT-----

@philderbeast
Copy link
Collaborator Author

Label merge+no rebase is necessary when the pull request is from an organisation.

@Mikolaj
Copy link
Member

Mikolaj commented Dec 1, 2023

Oh, yes, that's a known CI brittleness, I've done "Rerun failed jobs" from the CI screen, top-right corner (no idea what permission level is needed fro that).

Remove MIN_VERSION_base(4,7,0) as can't build anyway before ghc-8.4
@philderbeast
Copy link
Collaborator Author

The expected hashes were changed by @ulysses4ever in b2a36b7#diff-9ae45287f208ec36cc58ec4d1cef44d473e504c09fd24ff8d5eb6d58f1a83707 but I still think this pull request is worthwhile.

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

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

Good job, thank you!

@mpickering
Copy link
Collaborator

Why has the hash changed? (Which type in base has changed its representation?)

@ulysses4ever
Copy link
Collaborator

I don't think anyone had an idea how to investigate that when I asked on Matrix what to do about it when upgrading to GHC 9.8.

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Dec 7, 2023
@mergify mergify bot merged commit a1cbd89 into haskell:master Dec 7, 2023
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge+no rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants