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

Enforce UTF-8 encoding on reading and writing files #75

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

Jimbo4350
Copy link
Contributor

We were experiencing testing failures on Windows due to GHC 8.10.7's readFile not using UTF-8 by default. This caused golden file comparison errors when Unicode characters were present. For example:

478 ┃ testAllErrorMessages_
      479 ┃   :: forall a
      480 ┃    . (HasCallStack, Error a)
      481 ┃   => String
      482 ┃   -- ^ module name
      483 ┃   -> String
      484 ┃   -- ^ type name
      485 ┃   -> [(String, a)]
      486 ┃   -- ^ list of constructor names and values
      487 ┃   -> TestTree
      488 ┃ testAllErrorMessages_ = ErrorMessage.testAllErrorMessages_ goldenFilesPath
          ┃ │ Incorrect error message in golden file
          ┃ │ Reading file: test/cardano-api-golden/files/golden/errors\Cardano.Api.Fees.ScriptExecutionError\ScriptErrorEvaluationFailed.txt
          ┃ │ Golden test failed against the golden file.
          ┃ │ To recreate golden file, run with RECREATE_GOLDEN_FILES=1.
          ┃ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          ┃ │ 37c37
          ┃ │ <      Valid range: (-∞ , +∞)
          ┃ │ ---
          ┃ │ >      Valid range: (-Γê₧ , +Γê₧)

@@ -170,7 +174,9 @@ openFile filePath mode = GHC.withFrozenCallStack $ do
readFile :: (MonadTest m, MonadIO m, HasCallStack) => FilePath -> m String
readFile filePath = GHC.withFrozenCallStack $ do
void . H.annotate $ "Reading file: " <> filePath
H.evalIO $ IO.readFile filePath
liftIO $ IO.withFile filePath IO.ReadMode $ \handle -> do
Copy link
Contributor

@carbolymer carbolymer Nov 19, 2024

Choose a reason for hiding this comment

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

evalIO is needed for proper reporting of IO exceptions

Suggested change
liftIO $ IO.withFile filePath IO.ReadMode $ \handle -> do
H.evalIO $ IO.withFile filePath IO.ReadMode $ \handle -> do

Why this change is only in those two places ? I think we should use utf8 in all functions using string / text when accessing files:
appendFile, textWriteFile, textReadFile, fileContains

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you are right. However, it makes sense to accept this PR as an improvement to the library and do the other fixes in a future PR.

Copy link
Contributor

@carbolymer carbolymer Nov 20, 2024

Choose a reason for hiding this comment

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

do the other fixes in a future PR

Fine but let's do them before making a new release... And the version of hedgehog-extras with this change was tagged already. Can we try to limit technical debt we're introducing?

Or at least put an effort into making technical debt manageable and traceable by creating follow-up tickets/prs?

@newhoggy newhoggy requested a review from carbolymer November 20, 2024 11:30
@newhoggy newhoggy dismissed carbolymer’s stale review November 20, 2024 11:30

Recommend further PRs

@newhoggy newhoggy merged commit bdd89f7 into main Nov 20, 2024
16 checks passed
@Jimbo4350 Jimbo4350 deleted the jordan/enforce-utf8-golden-files branch November 20, 2024 13:55
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