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

Fix output truncation with json-errors and build failure. Fix #1160 #1199

Merged
merged 5 commits into from
Mar 12, 2024

Conversation

nwolverson
Copy link
Collaborator

Problematic situation is when --json-errors is passed (output is one very long line printed out all at once), there are many warnings (json > 65536 bytes) and there is an error (spago should exit with an error code). Set the error code and allow control flow to naturally exit to allow output to be flushed.

Description of the change

TODO

Checklist:

  • Added the change to the "Unreleased" section of the changelog
  • Added some example of the new feature to the README
  • Added a test for the contribution (if applicable)

P.S.: the above checks are not compulsory to get a change merged, so you may skip them. However, taking care of them will result in less work for the maintainers and will be much appreciated 😊

…ipt#1160

Problematic situation is when --json-errors is passed (output is one very long line printed out all at once),
there are many warnings (json > 65536 bytes) and there is an error (spago should exit with an error code).
Set the error code and allow control flow to naturally exit to allow output to be flushed.
@nwolverson
Copy link
Collaborator Author

Not clear why the test fails on windows only, I suspect something about trailing newline (I noticed there's some munging of that in the tests setup, and the output being single-line)

@f-f
Copy link
Member

f-f commented Mar 12, 2024

And about the Windows failure in CI: the fixture contains a "filename":"src/Main.purs", which becomes a "filename":"src\\Main.purs" on Windows.

When fixtures contain paths we just keep a different one around just for Windows, e.g. see the tests for spago sources:

spago [ "sources", "-p", "subpackage" ] >>= shouldBeSuccessOutput case Process.platform of
Just Platform.Win32 -> fixture "sources-subproject-output.win.txt"
_ -> fixture "sources-subproject-output.txt"

@@ -153,17 +151,20 @@ run opts = do
let reportTest = pedanticPackages || (fromMaybe false $ p.package.test >>= _.pedantic_packages)
Alternative.guard (reportSrc || reportTest)
pure $ Tuple p { reportSrc, reportTest }
unless (Array.null pedanticPkgs || opts.depsOnly) do
if Array.null pedanticPkgs || opts.depsOnly then do
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

well this clearly wasn't it

Copy link
Member

Choose a reason for hiding this comment

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

I guess it should be the other way around 😄

if Array.null pedanticPkgs || opts.depsOnly then do
  pure true
else do

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah that's what I meant by the comment but I still have 2 spago docs tests failing

Copy link
Member

Choose a reason for hiding this comment

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

Linux CI is happy!

@f-f f-f merged commit 10657ca into purescript:master Mar 12, 2024
3 checks passed
@nwolverson nwolverson deleted the many-errors-json-truncate branch March 13, 2024 09:13
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.

2 participants