Skip to content

cabal-install: update resolv to 0.2.0.2 #9059

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

Closed
wants to merge 1 commit into from

Conversation

jessicah
Copy link
Contributor

Fixes #9054


Please include the following checklist in your PR:

Bonus points for added automated tests!

@jessicah jessicah force-pushed the 3.10-resolv-0.2 branch 4 times, most recently from 71ac094 to 61e7e91 Compare June 23, 2023 00:40
@andreabedini andreabedini requested review from andreabedini and fgaz and removed request for andreabedini June 23, 2023 09:03
@jessicah
Copy link
Contributor Author

Also, grepping for DnsEncodeException and DnsDecodeException returned zero results, so these data constructors don't appear to be directly used. So the only user-visible change is the error string returned (seen in changes to test output).

@jessicah
Copy link
Contributor Author

Also, maybe one for #9008? :)

@@ -1,6 +1,8 @@
import Test.Cabal.Prelude

main = cabalTest $ withRemoteRepo "repo" $ do
skipIf "osx" =<< isOSX
Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/haskell/cabal/blob/6df9ab4f31061d9c87b17d61f58876752efb419a/cabal-testsuite/README.md#expect-tests

If you can't get the output of a test to be deterministic, no problem: just exclude it from recording and do a manual test on the output for the string you're looking for. Try to be deterministic, but sometimes it's not (easily) possible.

Since the warning is not part of the test expectations, instead of duplicating the test I think we can just add recordMode DoNotRecord, assertOutputContains "The index-state is set to 2022-01-28T02:36:41Z", and maybe assertOutputDoesNotContain "revert" to the first update call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fgaz I'll see what I can come up with, bit more than I bargained for :p Does that affect all lines in the test, or just for the first of the three tests?

Copy link
Member

Choose a reason for hiding this comment

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

Just the first

@fgaz
Copy link
Member

fgaz commented Jun 28, 2023

@Mergifyio backport 3.10

@mergify
Copy link
Contributor

mergify bot commented Jun 28, 2023

backport 3.10

🟠 Waiting for conditions to match

  • merged [:pushpin: backport requirement]

@fgaz
Copy link
Member

fgaz commented Jun 28, 2023

@jessicah could you retarget to master?

@fgaz
Copy link
Member

fgaz commented Jun 28, 2023

Oh, the bootstrap files are different. I'll open another pr with the changes I suggested

edit: according to the release guide we can defer the bootstrap update. i'll open another pr anyway since it looks like only the author can retarget the pr

@fgaz
Copy link
Member

fgaz commented Jun 28, 2023

@gbaz please have a look at #9069

@jessicah
Copy link
Contributor Author

@fgaz does this mean that this one can be closed, or is this still needed (with changes)?

@fgaz
Copy link
Member

fgaz commented Jun 29, 2023

We can close this

@fgaz fgaz closed this Jun 29, 2023
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