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

Code coverage support #154

Merged
merged 145 commits into from
Oct 21, 2022
Merged

Code coverage support #154

merged 145 commits into from
Oct 21, 2022

Conversation

xonixx
Copy link
Contributor

@xonixx xonixx commented Oct 6, 2022

This PR implements #144 and tries to address all CR notes in #144 (comment).

This PR is based on top of previous related PRs #148, #153 in order to eliminate the necessity for different hacky solutions.

I think code-wise this is ready. The only thing I still want to add - a documentation in own file coverage.md. While I'm working on this I would be really glad if you @benhoyt check the current implementation in this PR. Hopefully now it should be in much better shape than with prevous hacky approach.

xonixx added 30 commits August 15, 2022 22:41
This appears to be a bit tricky since this implementation processes all files as single awk source and does some validations (like making sure functions are defined). This is a problem if we want to annotate on per-file basis, since we need to parse files separately.
This appears to be a bit tricky since this implementation processes all files as single awk source and does some validations (like making sure functions are defined). This is a problem if we want to annotate on per-file basis, since we need to parse files separately.
This appears to be a dead end...

This reverts commit 3a3a6c2.

Revert "Work on annotating multiple files (multiple -f provided)."

This reverts commit fb42783.

Revert "Work on annotating multiple files (multiple -f provided)."

This reverts commit 5dbafa3.

Revert "Work on annotating multiple files (multiple -f provided)."

This reverts commit bb09b88.

Revert "Work on annotating multiple files (multiple -f provided)."

This reverts commit 3e0c70b.
@xonixx
Copy link
Contributor Author

xonixx commented Oct 20, 2022

Actually I managed to fix tests for windows. Still there is problem with macOS.

@benhoyt
Copy link
Owner

benhoyt commented Oct 20, 2022

Thanks! I'll do the final review later today. It looks like the macOS tests are failing, not because of this PR, but because homebrew gawk has been updated to a newer version (5.2.0), which seems to have something wrong with division? I'm not sure, I'll look into this further later.

@benhoyt
Copy link
Owner

benhoyt commented Oct 21, 2022

FYI, I resolve the merge conflicts after merging #155.

goawk.go Outdated Show resolved Hide resolved
goawk.go Outdated Show resolved Hide resolved
@benhoyt
Copy link
Owner

benhoyt commented Oct 21, 2022

Yeah, I think we should add a few basic tests for the -d* options -- good call. Feel free to do that in a subsequent PR, or open an issue for me to do it.

@benhoyt
Copy link
Owner

benhoyt commented Oct 21, 2022

Actually, I just opened an issue to track that: #156

interp/newexecute.go Outdated Show resolved Hide resolved
interp/newexecute_test.go Outdated Show resolved Hide resolved
internal/cover/cover.go Outdated Show resolved Hide resolved
internal/cover/cover.go Outdated Show resolved Hide resolved
internal/cover/cover.go Outdated Show resolved Hide resolved
testdata/cover/a1_covermode_set.awk Outdated Show resolved Hide resolved
@benhoyt
Copy link
Owner

benhoyt commented Oct 21, 2022

This is great, thank you. Just a couple of nit comments above, and then I'll merge this in. Looking forward to playing with AWK coverage! :-)

@xonixx
Copy link
Contributor Author

xonixx commented Oct 21, 2022

Just a couple of nit comments above, and then I'll merge this in.

Hopefully, done.

@benhoyt benhoyt changed the title Coverage Code coverage support Oct 21, 2022
@benhoyt benhoyt merged commit 7bcfd23 into benhoyt:master Oct 21, 2022
@xonixx xonixx mentioned this pull request Oct 22, 2022
benhoyt pushed a commit that referenced this pull request Oct 23, 2022
One more PR for #144 that addresses couple of minor things, missed in #154 

In this PR:

1. Remove unintuitive behavior (outputting annotated code when only `-covermode` flag set) - just use `-d` flag
2. Update cover.png screenshot: now we show if/for/while conditions in green (covered)
3. Fix for `./goawk -covermode=set 'BEGIN { print 123 }'` to not hang waiting for stdin. This started to happen because processing stops if `prog.Actions == nil` and we by mistake were changing it to empty slice during coverage annotation.
@benhoyt benhoyt mentioned this pull request Oct 24, 2022
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