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

Convert report using expression-coverage and not default coverage #20

Closed
parsonsmatt opened this issue Apr 15, 2024 · 5 comments
Closed

Comments

@parsonsmatt
Copy link

I'm investigating using this tool to get coverage information. However, the default view is giving a somewhat misleading picture into our code coverage: record fields and derived type class instances are reporting as red. hpc reports have an option to show the "expression-level" coverage, which seems to be much more useful to me.

Is it possible to have hpc-codecov generate a Codecov report using this expression-level coverage?

@8c6794b6
Copy link
Owner

The information you mentioned as "expression-level" coverage is already in use in hpc-codecov, I suppose.

Can you provide some sample codes to describe that the hpc report gives useful info but hpc-codecov gives a misleading picture?

Regarding record fields and derived instances, the following codes shall illustrate my current understanding of how the report generated via hpc markup command works:


{-# LANGUAGE GeneralizedNewtypeDeriving #-}
--  File: field_and_derived.hs
--
-- An example to show the coverage of record fields and derived instances in
-- HTML files generated via the 'hpc markup' command. Compile, run, and generate
-- coverage report with:
--
-- > $ ghc -fhpc -o a.out field_and_derived.hs
-- > $ rm -f a.out.tix && ./a.out
-- > $ hpc markup a.out.tix
--
-- And observe the generated 'Main.hs.html' file. Tested with ghc 9.6.4 and hpc
-- 0.68.

module Main where


-- ---------------------------------------------------------------------------
-- Coverage of record fields
-- ---------------------------------------------------------------------------

-- To tell the hpc to mark a record field to be covered, one needs to evaluate
-- the record field as a function. Using the field in a pattern match does not
-- trigger hpc to cover the field.

-- Simple data type containing two fields, 'field1' and 'field2'.
data R = R
  { field1 :: Bool
  , field2 :: Bool
  }

-- A function taking 'R' and printing its contents. The 'field1' is used in the
-- pattern match, but not used as a function. On the other hand, the 'field2' is
-- not used in the pattern match, but used as a function.
runField :: R -> IO ()
runField (r@R {field1=one}) = do
  print one
  print $ field2 r


-- ---------------------------------------------------------------------------
-- Coverage of derived instances
-- ---------------------------------------------------------------------------

-- To tell the hpc to mark the type class name of the derived instance, one need
-- to evaluate /the last member function/ of the class. Evaluating the member
-- functions other than the last one won't affect the coverage of the type class
-- name in the deriving clause. I am not sure whether this behavior is intended
-- or not.
--
-- For example, the (simplified) definition of the 'Functor' class from GHC.Base
-- (base 4.19.0.0) is:
--
-- > class Functor f where
-- >   fmap :: (a->b) -> f a -> f b
-- >   (<$) :: a -> f b -> f a
-- >   (<$) = fmap . const
--
-- in this case, when the '(<$)' is evaluated for the derived instance, the
-- 'Functor' in the 'deriving (...)' clause will be marked as covered.
--
-- Another example, is 'Applicative' from GHC.Base:
--
-- > class Functor f => Applicative f where
-- >   pure :: a -> f a
-- >   (<*>) :: f (a->b) -> f a -> f b
-- >   (<*>) = liftA2 id
-- >   liftA2 :: (a->b->c) -> f a -> f b -> f c
-- >   liftA2 f x = (<*>) (fmap f x)
-- >   (*>) :: f a -> f b -> f b
-- >   a1 *> a2 = (id <$ a1) <*> a2
-- >   (<*) :: f a -> f b -> f a
-- >   (<*) = liftA2 const
--
-- in this case, whether '(<*)' is evaluated or not is the only thing considered
-- to cover 'Applicative' in the deriving clause.
--
-- And (simplified) definition of 'Monad', again from GHC.Base:
--
-- > class Applicative m => Monad m where
-- >   (>>=) :: m a -> (a -> m b) -> m b
-- >   (>>) :: m a -> m b -> m b
-- >   m >> k = m >>= \_ -> k
-- >   return :: a -> m a
-- >   return = pure
--
-- in this case, 'return' is the one.

-- Sample newtypes 'A' and 'B', to compare the coverage of class names in the
-- deriving clause.

newtype A f x = A {unA :: f x} deriving (Functor, Applicative, Monad)

newtype B f x = B {unB :: f x} deriving (Functor, Applicative, Monad)

-- A function to evaluate instance methods of 'A' and 'B'.
--
-- For 'A', all the member functions of 'Functor', 'Applicative', and 'Monad'
-- except for the last one in each class are evaluated.
--
-- For 'B', on the other hand, only the last member functions of 'Functor',
-- 'Applicative', and 'Monad are evaluated, which are '(<$)', '(<*)', and
-- 'return'.
runDerived :: IO ()
runDerived = do
  print $ unA $ fmap not (A (Just False))
  print $ unA $ liftA2 (*) (pure False *> pure 7) (pure succ <*> (A (Just 5)))
  print $ unA $ A (Just False) >>= \x -> A (Just ()) >> A (Just (not x))
  --
  print $ unB $ False <$ B (Just True)
  print $ unB $ B (Just True) <* B (Just "foo")
  print $ unB $ (return True :: B Maybe Bool)


-- ---------------------------------------------------------------------------
-- The main
-- ---------------------------------------------------------------------------

main :: IO ()
main = do
  runField (R True False)
  runDerived

@parsonsmatt
Copy link
Author

Right - so if I look at hpc_index_exp.html, then we get this:

image

The "Top Level Definitions" shows 59% code coverage. But "Expressions" has 94% code coverage. "Expressions" is a more useful thing to actually track.

Consider the derived instances:

image

There is a penalty for deriving {Functor,Applicative,Monad} A, even though the methods are being used - just not all of them. Derived instances aren't really worth testing (IMO), because they're "obviously correct."

Now, if we had written:

instance Functor f => Functor (A f) where
  fmap f (A fx) = A (fmap f fx)

Then A (fmap f fx) would count as an expression, and you'd need to ensure it was exercised in test code in order to avoid the penalty. This makes sense, because a custom instance may easily have bugs. But a derived instance won't.

Likewise, consider the record field:

image

Here we are penalized because we used a record field as a pattern match. If I add a record construction that uses the field, we're still penalized:

image

Writing tests that exercise record field selectors as functions is a complete waste of time, and the "Expression" report seems to ignore these failures.

So what I am interested in is determining how we can make Codecov report coverage on expressions, not on the top-level declarations.

@8c6794b6
Copy link
Owner

Continuing with field_and_derived.hs code, the output from hpc-codecov command generates Codecov's JSON format data. At the moment, partially covered lines constantly have "1/2" hits in this format.

$ ghc -fhpc -o a.out field_and_derived.hs
$ rm -f a.out.tix && ./a.out
$ hpc-codecov a.out.tix | jq
{
  "coverage": {
    "field_and_derived.hs": {
      "28": 0,
      "29": 1,
      "36": 1,
      "37": 1,
      "38": 1,
      "92": "1/2",
      "94": "1/2",
      "105": 1,
      "106": 1,
      "107": "1/2",
      "108": "1/2",
      "110": "1/2",
      "111": "1/2",
      "112": 1,
      "120": 1,
      "121": 1,
      "122": 1
    }
  }
}

I interpret your concern as removing the entries for lines 28, 92, and 94.

The line 28 is where field1 is defined, which is marked as missed in above JSON.

Lines 92 and 94 contain the definition of newtype A and B, which is marked as partial. This is because some of the member functions in Functor, Applicative, and Monad are not evaluated. The hpc-codecov command will mark the line containing the type class name as covered when all of the member functions of the type class are evaluated. This behavior is different from hpc markup command, which marks the derived class name as covered when the last member function is evaluated, as I wrote in the comment of field_and_derived.hs.

Lines 107, 108, 110, and 111 contain unevaluated expressions False, (), True, and "foo". These lines are marked as
partial.


What I can think of is to add two options: one to ignore the hit count of TopLevelBox entries, and another to ignore record fields.

By ignoring TopLevelBox, the type class names in deriving clauses will not marked as missed code. Top-level functions are not considered as part of the source code to take coverage.

To ignore record fields, need to detect the corresponding ExpBox entries. For field1, there is a missed ExpBox entry in the .tix file:

$ hpc show a.out.tix | grep 28:5
   90          0 Main                           28:5-28:10 ExpBox False
   91          0 Main                           28:5-28:10 TopLevelBox ["field1"]

Seems like, if two sequential entries had the same start and end positions, and the first of them being ExpBox and the second being TopLevelBox, the ExpBox entry could be considered as a record field.

8c6794b6 added a commit that referenced this issue Apr 25, 2024
Add two new options mentioned in #20. The "--expr-only" option counts
expression only, and "--ignore-dittos" ignores source codes possibly
generated by the compiler.

Add some tests for new options, add some examples README.
8c6794b6 added a commit that referenced this issue Apr 25, 2024
Add two new options mentioned in #20. The "--expr-only" option counts
expression only, and "--ignore-dittos" ignores source codes possibly
generated by the compiler.

Add some tests for new options, add some examples README.
8c6794b6 added a commit that referenced this issue Apr 26, 2024
Add two new options mentioned in #20. The "--expr-only" option counts
expression only, and "--ignore-dittos" ignores source codes possibly
generated by the compiler.

Add some tests and test data files for new options, add some examples
README.
8c6794b6 added a commit that referenced this issue Apr 26, 2024
Add two new options mentioned in #20. The "--expr-only" option counts
expression only, and "--ignore-dittos" ignores source codes possibly
generated by the compiler.

Add some tests and test data files for new options, add some examples
README.
8c6794b6 added a commit that referenced this issue Apr 26, 2024
Add two new options mentioned in #20. The "--expr-only" option counts
expression only, and "--ignore-dittos" ignores source codes possibly
generated by the compiler.

Add some tests and test data files for new options, add some examples
README.
@8c6794b6
Copy link
Owner

@parsonsmatt Added two new options, --expr-only and --ignore-dittos.
The new options are available in hpc-codecov-0.6.0.0.
Also, hpc-codecov-action@v4 is updated to take inputs for the new flags.

@parsonsmatt
Copy link
Author

Thank you! That's amazing.

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

No branches or pull requests

2 participants