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 coverage, restore defun advice #1151

Merged
merged 3 commits into from
Feb 27, 2023
Merged

Fix coverage, restore defun advice #1151

merged 3 commits into from
Feb 27, 2023

Conversation

sirlensalot
Copy link
Contributor

@sirlensalot sirlensalot commented Feb 25, 2023

  • Fix inlining in TC for coverage
  • Restore defun coverage
  • Add test coverage

@@ -418,6 +418,7 @@ test-suite hspec
Analyze.TimeGen
Analyze.Translate
ClientSpec
CoverageSpec
Copy link
Contributor

Choose a reason for hiding this comment

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

Love that you added a test.

(ref,adv) <- mkCoverageAdvice
s <- set (rEnv . eeAdvice) adv <$> initReplState (Script False fn) Nothing
void $! execScriptState' fn s (set (rEnv . eeAdvice) def)
writeCovReport ref
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you testing only that it can be generated, but not the contents emitted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See latest update, it's a golden test

Copy link
Contributor

@imalsogreg imalsogreg left a comment

Choose a reason for hiding this comment

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

Looks great!

@emilypi emilypi merged commit de9b0c1 into master Feb 27, 2023
@emilypi emilypi deleted the feat/fix-coverage branch February 27, 2023 19:14
@emilypi emilypi mentioned this pull request May 12, 2023
8 tasks
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.

4 participants