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

WIP More precise coverage #11802

Closed
wants to merge 1 commit into from
Closed

WIP More precise coverage #11802

wants to merge 1 commit into from

Conversation

carnaval
Copy link
Contributor

Make coverage depends on lowered code statement number instead of text
line numbers. The reported number is the minimum count of every statements
mapping to the relevant line.

        - function f(b)
        1     b && (x = 4)
        1     x = 2
        - end

        - f(true)
        - function f(b)
        0     b && (x = 4)
        1     x = 2
        - end

        - f(false)

just hacked this, not tested at all, consider this my argument against #11792 :-)

@timholy
Copy link
Member

timholy commented Jun 21, 2015

👍

@ScottPJones
Copy link
Contributor

Hey, you won't get any argument from me! I did #11792 just because people said the by line coverage couldn't be fixed...
This is exactly what I wanted originally!
👍 💯

Make coverage depends on lowered code statement number instead of text
line numbers. The reported number is the minimum count of every statements
mapping to the relevant line.
@@ -1134,8 +1134,10 @@ const jl_value_t *jl_dump_function_asm(void *f)

typedef std::map<std::string,std::vector<GlobalVariable*> > logdata_t;
static logdata_t coverageData;
// map file => pc => line no
Copy link
Contributor

Choose a reason for hiding this comment

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

what's pc ?

Copy link
Contributor

Choose a reason for hiding this comment

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

pc = program counter = ip = instruction pointer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah the comment is wrong now also :-) and the code a bit ugly, I'll do a cleanup pass tomorrow

@StefanKarpinski
Copy link
Member

This seems great. Does reporting fractional coverage make sense? I.e. 0.5/1 or something like that.

@JeffBezanson
Copy link
Member

Love it!

@kshyatt
Copy link
Contributor

kshyatt commented Jun 22, 2015

💯 !

@mschauer
Copy link
Contributor

How about reporting the range of counts so "partial bottlenecks" are visible (bottlenecks constraint to parts of the line).

@ScottPJones
Copy link
Contributor

That sounds like a good idea @mschauer

@tkelman
Copy link
Contributor

tkelman commented Jun 23, 2015

Right now the .cov format is sort of designed for reporting a single number, right? So either we would have to extend that format to report additional information, or output multiple copies of the file for different choices of a single number to report?

@ScottPJones
Copy link
Contributor

I'd figured a simple extension would be done, like min:max instead of count.
BTW, what is done to handle multiple processes trying to write coverage information at once?

@tkelman tkelman mentioned this pull request Jun 26, 2015
14 tasks
@IainNZ
Copy link
Member

IainNZ commented Jul 3, 2015

So we now have codecov.io support, which can track partial coverage. Just to kinda incentivize getting this over the line.

@waldyrious
Copy link
Contributor

So we now have codecov.io support

Noob comment here, but doesn't that mean something should be visible on this page? Is it just a matter of time, or am I looking at the wrong place?

@IainNZ
Copy link
Member

IainNZ commented Jul 3, 2015

Did anyone say it should be? For now its living at https://codecov.io/github/kshyatt/julia?ref=master

@kshyatt
Copy link
Contributor

kshyatt commented Jul 3, 2015

So the buildbots aren't running right now, @waldyrious, for a variety of reasons (ask @tkelman or @staticfloat for more). I've been running coverage on my fork of Julia for now. If anyone would like some help getting a copy of Julia coverage running on their own machines, drop me a line any time.

@waldyrious
Copy link
Contributor

Did anyone say it should be? For now its living at https://codecov.io/github/kshyatt/julia?ref=master

I fully admitted that I could be looking at the wrong place. Thanks for the link.

@kshyatt thanks for the clarifications :)

@jiahao
Copy link
Member

jiahao commented Feb 7, 2016

bump

@Sacha0 Sacha0 mentioned this pull request Jun 14, 2016
@Keno
Copy link
Member

Keno commented Feb 28, 2020

This is somewhat stale, but @vtjnash has done some work on more precise coverage recently, which I think had roughly the same effect.

@Keno Keno closed this Feb 28, 2020
@vtjnash
Copy link
Member

vtjnash commented Feb 29, 2020

This is a significantly different piece of design from my recent work. Though I agree we probably can’t use the PR in its current state.

@DilumAluthge DilumAluthge deleted the ob/cov branch March 25, 2021 22:12
@DilumAluthge DilumAluthge restored the ob/cov branch March 25, 2021 22:12
@DilumAluthge DilumAluthge deleted the ob/cov branch March 25, 2021 22:12
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.