Fix #9151 (cabal init logs at loglevel "Log")

[aleeusgr]

The actual ADT for severity should change because "Log" is useless as a datatype, arguably it shouldn't even be an entry in the ADT and the severity should be at Info for everything that's not a Warn.

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
• The documentation has been updated, if necessary.
• Manual QA notes have been included.

QA notes:

when calling cabal init the user sees log entries categorized as Info, while entries categorized as Log should not appear.

[andreabedini]

I am in with this change. I left a couple of comments.

[geekosaur]

Not repeating the points that have already been raised.

[aleeusgr]

Validation test is broken.

Todo:

• what's a validation test?
  Validation testing is a process of evaluating software during or at the end of the development process to ensure that it meets specified business requirements.
• does it have to do with where I placed the function?
• is it something that will resolve itself?

[emilypi]

@aleeusgr Re: validation tests - i'll take a look in a bit to remind myself.

[aleeusgr]

OK. Meanwhile I'll try to run the failing test locally in 9.2.8

[aleeusgr]

All checks pass. Maybe guard expression caused validate test to fail.

@emilypi please check QA note I added at the top of this PR.
I will add the change suggested by @ulysses4ever unless there are objections.

I need to

• rebase the PR to the latest of master branch.
• Think about testing strategy
• wait for second approval
• add squash and merge label.

[Mikolaj]

@aleeusgr: don't hesitate to mark the PR with help_needed and ping the Matrix channel if you feel blocked.

[aleeusgr]

Hi @Mikolaj
I think we did everything that was planned here and checks pass.

I was waiting for the second approval, I don't know how things work here: I assumed someone would ping me if any actions are needed.

@emilypi could you confirm we achieved our goals for this PR?

Thanks.

[Mikolaj]

I was looking at your checklist in the comment above. Any ideas for an automated test?

[aleeusgr]

Maybe we could parse the log output line by line and fail if [Log] shows up?
@Mikolaj is this a sound thing to do?

[Mikolaj]

Sounds good to me. Perhaps also check the test output is not empty (and make sure the cabal-install commands issued in the test really should log something). Alternatively, there are golden tests somewhere (see the READMEs in the test folders and also CONTRIBUTING) so, if they apply to this situation, you can ensure the log output is always exactly the same as you record and accept initially. I suppose cabal init logs are deterministic, so that may work.

[aleeusgr]

• write/find a test that interacts with log output of cabal-install
• look at suites:
  • unit tests
  • integration tests
  • long tests

[aleeusgr]

• review golden tests
  golden test outputs are stored in https://github.com/haskell/cabal/tree/master/cabal-install/tests/fixtures/init/golden
  The golden tests are in https://github.com/haskell/cabal/blob/master/cabal-install/tests/UnitTests/Distribution/Client/Init/Golden.hs

what is a golden test ?
Golden test is like a unit test but its output is stored in a file.

Golden tests define 6 test groups which correspond(?) to this comment:

cabal/cabal-install/tests/UnitTests/Distribution/Client/Init/Golden.hs

Lines 44 to 48 in 6314590

	-- * Empty flags, non-simple target gen, no special options
	-- * Empty flags, simple target gen, no special options
	-- * Empty flags, non-simple target gen, with generated comments (no minimal setting)
	-- * Empty flags, non-simple target gen, with minimal setting (no generated comments)
	-- * Empty flags, non-simple target gen, minimal and generated comments set.

five lines vs 6 test groups. The first group is one ring to rule them all:

cabal/cabal-install/tests/UnitTests/Distribution/Client/Init/Golden.hs

Lines 61 to 67 in 6314590

	"golden"
	[ goldenLibTests v pkgIx pkgDir pkgName
	, goldenExeTests v pkgIx pkgDir pkgName
	, goldenTestTests v pkgIx pkgDir pkgName
	, goldenPkgDescTests v srcDb pkgDir pkgName
	, goldenCabalTests v pkgIx srcDb
	]

[aleeusgr]

https://github.com/haskell/cabal/tree/master/cabal-install/tests#integration-tests

each test is a shell script.
🤔
But I don't see any .sh files, for example cabal-install/tests contains no shell scripts.

Looking at the file, it contains only test trees but no test code.

The list of unit tests can be found in cabal-install.cabal

The functionality I added relates to Init command, so I should look at

cabal/cabal-install/tests/UnitTests.hs

Line 13 in 6314590

import qualified UnitTests.Distribution.Client.Init

There are five test trees(?) imported in init: FileCreators, Interactive, Non-interactive, Golden, Simple.

[aleeusgr]

• look at the tests that live in cabal-testsuite/PackageTests
• review https://github.com/haskell/cabal/tree/master/cabal-testsuite#how-to-write
  • build cabal-testsuite?

to run tests:
nix develop github:yvan-sraka/cabal.nix

Parsing log outputs:
https://stackoverflow.com/questions/42702618/parsing-a-log-file-in-haskell
https://dev.to/anaynayak/haskell-parsing-a-log-message-1290
https://hackage.haskell.org/package/attoparsec

[emilypi]

What you could do is probably add a parameter to add a log queue accumulator in the prompt state. I.e.

newtype PurePrompt a = PurePrompt
  { _runPrompt
      :: NonEmpty String
      -> Either BreakException (a, NonEmpty String)
  }
  deriving (Functor)

Should turn into the following:

type LogQueue = [Text]

newtype PurePrompt a = PurePrompt
{ _runPrompt
    :: NonEmpty String
    -> Either BreakException (a, NonEmpty (LogQueue, String))
}
deriving (Functor)

Then the instances will need to be adjusted so that dedicated log functions are used which pipe to the correct behaviors in the IO case and the PurePrompt case. This is extracurricular. I think if you establish that the log output is doing the correct thing in a screenshot and raise an issue to track logging in PurePrompt state, I'll accept the PR.

[aleeusgr]

[emilypi]

Great job @aleeusgr :)

Congrats on contributing to Cabal!

[aleeusgr]

Thank you @emilypi
It's a pleasure 😀

[aleeusgr]

dear @geekosaur or @emilypi

I regret to inform you that there is a problem with merging this PR:

I wonder what I could do to make it go through but it looks I am powerless in this situation.

Thank you for your attention

[Mikolaj]

Strange. Let me rebase.

[Mikolaj]

@mergify rebase

[mergify]

rebase

✅ Branch has been successfully rebased

[Mikolaj]

Success! It must have been a mergify hiccup (and then a GHA random hangup, which I restarted).

@aleeusgr: congrats again! :)