List of changes:

• linkify bug numbers
• removes authors' emails (instead of using substring)
• pushes with net-new coverage
• allow requesting more changesets (button at bottom saying "load more pushes")
• spinner when loading the main view

Please add a column to show the push date of each of the central pushes. https://firefox-code-coverage.herokuapp.com/#/

Checkout the releng services repo:
git clone https://github.com/mozilla-releng/services

In one tab:
./please run postgresql
on another:
./please run shipit-uplift
To see all available APIs:
https://localhost:8011/docs

For now we can run "yarn test" which runs "yarn lint && yarn build".

To do

• load code coverage status for each m-c push

Blockers

• Fixed - mozilla/release-services#594
• New - Use a worker to process coverage API requests

Notes

Plan disccused here:
https://public.etherpad-mozilla.org/p/code_coverage_Q3_17

I'm pasting over here some conversations we've had around the topic.

Armen wrote:

We will only be able to show accurate code coverage data for the latest version of a file
and/or test. If there are multiple changesets on a m-c push touching the same file or
related tests we will not be able to give accurate code coverage information for the older
changesets. Did I understand correctly?

gmierz wrote:

A strategy for getting around the multiple change sets for a single file could be to mark
what changeset each of the changes come from. That way, if we just use the latest file
in the current m-c push and the latest in the previous m-c push, there should be no
issues. But you are right to say "only" because this doesn't necessarily give us coverage
data from those patches exactly. I think this would be the best way to get around the
problem for now though.

What we hope to be able to use, ideally, to fix this is changeset algebra. Kyle, and I have
been playing with this idea and he's made a nice data structure that can take code
coverage backwards in changesets. I'll write down an example to illustrate this to you
tonight because it's not easy to explain in words. And we'll have to wait before we can
use it so we can test this idea a bit more.

Marco wrote:

Yes, this is correct. In some cases, we might be able to also say something about the
intermediate commits. For example, if commit A and commit B both change the same
file but not the same lines, then we can try to find the lines modified by A in the last
revision by taking into account the additions/removals made by B.

In the case where they both modify exactly the same line, we only really care about
the end result. If the line is covered, then it doesn't matter if it wasn't covered in A,
as long as it is covered in the end. If the line is not covered, then we will blame both A
and B as introducing an uncovered line.

Kyle wrote:

For our current implementation plan this understanding is correct, but it is not correct
in general: Code coverage can be mapped from one revision to another; even in the
case of multiple changes to the same file: The solution is to treat files as points in a
vector space; convert changesets to (orthogonal, binary) matrices; which allows us
to map coverage vectors from one revision to another, forward or backward.
We use that coverage to either make claims about total coverage, or claims about
coverage on net new lines, or combine multiple coverage runs for a more-stable
coverage statistic.

Kyle wrote:

The math is just a tool that provides the best conclusion given the available data. ... the math
I am proposing can handle multiple changes to the same file just fine. I added specific
examples [1] as tests so that Greg may see it operate.

... it is the best way to simulate per-changeset coverage without actually doing it. It also
provides us a way to compare coverage over multiple revisions. Again, neither of these
capabilities may be useful, which is why we are putting Marco's coverage mapping (and the
math that can replace it) behind an API [2] the frontend can use.  We can decide later if we
want the features [that the] math provides.

[1] https://github.com/klahnakoski/diff-algebra/blob/master/tests/test.py

Right now, you have to reload the page to see the results.

Show them collapsed.

For example .ini and .py files.

https://localhost:8011
vs
https://uplift.shipit.staging.mozilla-releng.net

Currently we have appError, csetMeta, parsedDiff and we do data manipulations across all modules.
Instead I want to do all coverage and parsedDiff manipulations in DiffViewerContainer, plus store new data. The data structure will be like this:

{
  appError // as -is
  csetMeta // XXX: We will move information from 'coverage' into 'files'
  files: {
    filePath: {
       // The next two are to determine which files score worst and sort by it
      addedLines,
      coveredLines,
      folded, // To support folding files
    }
  }
}

The diff viewer should not give any color to the removed lines because they are not important for determining coverage. Maybe the removed lines should not even be shown; instead showing only the new hunks and coloring the based on if they are covered, or not

To do:

• add coverage summary per diff and per file

For backouts, the coverage information is not important.

This will help RelMan when viewing the main page of changesets; they are only interested in low coverage. 0/0 does not fit their definition of low coverage

This cset [1] has a bug at the end of the page.
There's 4 lines added.
The coverage data [2] only has data for 2 lines.
I was expecting another 2 lines with '?'

@marco-c is this a bug in the backend? or should I mark lines w/o coverage as white?

[1] https://firefox-code-coverage.herokuapp.com/#/changeset/c56352d90f678c2ad104da657aae116513502153
[2] https://uplift.shipit.staging.mozilla-releng.net/coverage/changeset/c56352d90f678c2ad104da657aae116513502153

    {
      "changes": [
        {
          "coverage": "Y",
          "line": 170
        },
        {
          "coverage": "Y",
          "line": 385
        }
      ],
      "name": "layout/build/nsLayoutStatics.cpp"
    },

"I think one of the features we definitely want to have is a way to sort the changesets by coverage level (showing changesets with low coverage first)."

The second file is /dev/null and should be /dom/base/nsGlobalWindow.cpp

[1] https://firefox-code-coverage.herokuapp.com/#/changeset/862355bc5c589cc22420526d5bbf10e7f6f7ba78

@marco-c CC

First of all, this is a non-trivial amount of work.

The easiest implementation would be to simply collapse changesets associated to 1 bug yet still be able to show each individually and reach the diff viewer for each changeset.

A perfect example of which cases would be benefit the most from this work would be a push like the one in the screenshot below (15+ changesets associated to 1 bug).

I'm not planning to tackle this, however, I'm open to mentor anyone interested on this project.

So I can test deployments of a GH PR.

Please change the routing to the diffviewer so that the view ("diff") is explicit, and the changeset is a parameter. Instead of

https://firefox-code-coverage.herokuapp.com/#/changeset/b2232cf95183b220c1918798e615a3591724f5c8

use

https://firefox-code-coverage.herokuapp.com/diff?changeset=b2232cf95183b220c1918798e615a3591724f5c8

A lot of data manipulations happen at the lower components rather than at the top component.

Here's a comment from the PR (paraphrased):

Calculate all of this information at the top level component where the data is fetched.
Create a data structure like this (There might more data needed for each line):

this.state = {
   selectedLine: 0,
   lines: {
     0: {
       testList: getTestList(coverage.testsPerHitLine[lineNumber],
     }
   }
   allTests: getTestList(coverage.allTests),
}

That way all data processing happens in a single place and each piece in the top state has a direct influence on the UI. Your UI is a representation of your state.

No more data manipulations at the low level sub-components.

Also, these changes make it very easy to access all significant data of a line anywhere in the app like this this.state.lines[lineNumber].testList or this.state.allTests.

• metadata
• list of files with links to the divs
• collapsible divs
• linkable lines
• receive changeset parameter to display and load on the fly
• when loading a raw-diff show a spinner (since fetching takes long)
• when a diff gets loaded once see if we can cache it (probably with react-redux)
• remove new line if value of old one is the same
• receive changeset parameter to display and load on the fly

Loading the page now:

There are a total of 391 changesets. This is likely too many changesets, users will not be able to look at all of them.

Add this information to the docs.

The added lines which are not covered are either:

• not instrumented because they are not executable (e.g. a comment, or a variable declaration);
• not instrumented because they are optimized out by the compiler;
• belong to files for which we don't collect coverage (e.g. Python files, INI files, and so on);
• a bug somewhere in the instrumentation or the parsing of the results.

The first case is not actually a problem (the line is not executable at all, so we shouldn't consider it for coverage measurement purposes).
The second case can be a problem if it happens often (it would be interesting to compare a coverage report coming from a build without optimizations with a coverage report coming from a normal build).
The third case is not actually a problem, but to avoid noise perhaps we can collapse by default the hunks that modify files for which we don't collect coverage.
The fourth case hopefully never happens, but we can't know for sure.

Move "GitHub", "Codecov" and "Coverage backend" links to the bottom of the page and make them very small.
They're only useful for contributors of this app when debugging.

Wrap fetch() in a Promise; add a setTimeout and call reject() if it goes off, otherwise resolve with the result of the fetch()

There is propTypes but I'm going to try Flow.
The fx team is moving to it.

I've wasted half a day because I was trying to use a variable that was not defined.
The error in the console was TypeError: Function.prototype.bind.apply(...) is not a constructor

Calculating new_lines_covered/total_new_lines may be wrong: The new lines may not be on executable files, so should not be part of the count. The UI should intersect the new lines of the changeset with the uncovered lines from the coverage to get uncovered_new_lines, which then can be used to calculate the percent new lines covered.

If you edit the address bar:
http://localhost:5000/#/changeset/112dba6948d388a6154f073c4f09c2c6a9addcf7
change the node id and press "enter" only some parts of the UI will update.

This is minor but should be fixed.

This might have to do with componentDidMount() not being called.

CC: @LinkaiQi @klahnakoski @armenzg

This viewer would show the coverage, from multiple tests, for a given file, at a given revision.

The user should be able to interact with the UI to know what test covered what lines.

Based on discussions with Kyle,

• We intend to get the sources from hg. Here is an example:
  https://hg.mozilla.org/integration/mozilla-inbound/raw-file/6b0491f83229/accessible/atk/Platform.cpp

• We can get a list of the latest coverage artifacts from ActiveData (coverage sticks around for 3 weeks, so we will use this to get new revision numbers when the old ones expire):
  https://activedata.allizom.org/tools/query.html#query_id=70sYgySl

• We can look at the coverage for a particular source file; each record is from a chunk of a test suite (aka "a test"): https://activedata.allizom.org/tools/query.html#query_id=WdGo1H+i

There is a drawing of this idea in the docs directory.

We cannot parse this changeset
https://firefox-code-coverage.herokuapp.com/#/changeset/3acb30b37718b0abc626416b9554acbc880a0078
https://hg.mozilla.org/mozilla-central/rev/3acb30b37718b0abc626416b9554acbc880a0078

@jmaher CC

I am viewing:
https://fx-codecov-staging-pr-25.herokuapp.com/#/changeset/9a26478b903485d0f70b7a24fec0a7fe2d0c124f

I click on the "code coverage backend":
https://uplift.shipit.staging.mozilla-releng.net/coverage/changeset/9a26478b903485d0f70b7a24fec0a7fe2d0c124f

and I get:

The current JSON agreed metadata will look like this:

    {
      "child_coverage_revision": <Newer-Coverage-Revision>,
      "parent_coverage_revision": <Older-Coverage-Revision>,
      [
        {  // Source file 1
          "name": <Source-file-name>,
          // "extra": { .. },     // extra information if we ever need some. Will only be added once we start needing it.
          "changes": [      // changes broken down by hunk
              {  // Hunk 1
                "old-start, old-length, new-start, new-length": // This is the field that is shown in hg as "@@ old_line_start, old_length, new_line_start, new_length @@", we can just store the entire string
                "lines": [
                  (Coverage:true/false, Line_type: any("+", "-", ""), Old_line, New_line, Content), // Where 'any' signifies that either of those string options can be given.
                  ...
                ]
              },
              {  // Hunk 2
                ...
              }
          ]
        },
        ...
        {  // Source file 2
           ...
        }
      ]
    }

We can either use a modal [1] or some info in a div.

@calixteman any preference?

[1] https://github.com/armenzg/readable-app/blob/master/src/components/post.js#L142-L160

Given a changeset show the diff.

Sometimes it would be nice to know that the UI is just waiting for data to load.
This can easily be accomplished by returning a string ("Loading...") when the components first load before the fetch requests have completed.

This servo diff makes the parser fail.

I'm thinking of switching to this package (since it is more actively maintained) and see if it gets fixed.

The full error can be seen on a development environment:
http://localhost:5000/#/changeset/112dba6948d388a6154f073c4f09c2c6a9addcf7

Uncaught (in promise) Error: Objects are not valid as a React child (found: TypeError: Cannot read property 'changes' of null). If you meant to render a collection of children, use an array instead or wrap the object using createFragment(object) from the React add-ons. Check the render method of `DiffViewerMeta`.
    at invariant (invariant.js:44)
    at traverseAllChildrenImpl (traverseAllChildren.js:144)
    at traverseAllChildren (traverseAllChildren.js:172)
    at flattenChildren (flattenChildren.js:66)
    at ReactDOMComponent._reconcilerUpdateChildren (ReactMultiChild.js:202)
    at ReactDOMComponent._updateChildren (ReactMultiChild.js:310)
    at ReactDOMComponent.updateChildren (ReactMultiChild.js:297)
    at ReactDOMComponent._updateDOMChildren (ReactDOMComponent.js:942)
    at ReactDOMComponent.updateComponent (ReactDOMComponent.js:760)
    at ReactDOMComponent.receiveComponent (ReactDOMComponent.js:722)

I'd like to have a link on the top of the page to bugzilla with correct bug number to add a pre-filled comment with coverage numbers.

These commits contain hundreds of changes, it's difficult for a user to understand which change is being introduced by which patch and so it's difficult to blame a single patch for introducing uncovered code.

CC: @yuenj @klahnakoski @armenzg

The revision numbers will be showed in a sidebar on the main view.
The revision number will be clickable.
User should be able to interact with the sidebar to choose which revision she/he wants to browse on

I'm hitting CORS left and right. I need backend support for the data I need.

For instance the data contained in here:

• https://codecov.io/api/gh/marco-c/gecko-dev/commits

I would also need data from Mapper to relate Git commits to Hg commits.
OR load the raw diffs from GH directly

I've tried setting up shipit without success.

Once we're satisfied with the product we should verify the following changesets:

• Rust diff
  • Since we don't support Rust we should see a diff without highligting

This will coordinate all three fetches happening to end at the same time:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all

The filenames of the diff viewer are too small, make it bigger. so it overwhelms the less relevant "@@" chunk markers.

It's painful to find the uncovered added lines (need to scroll a lot).

Instead of iterating by what we receive from Hg web [1] let's look at the contents of coverage.diffs[diffBlock.from] first.

diffBlock.from is a filePath.
coverage.diffs[diffBlock.from] contains an object with a property per line:

   { 413: 'Y', 414: 'N', 415: '?'   }

[1] https://github.com/armenzg/firefox-code-coverage-frontend/blob/master/src/components/diffviewer.js#L76

List of commits. Linking to diff viewers.

To do:

• fetch hg web data and display changesets
• description
• remove ffxbld from list of commits (2017-09-05)
• showing tipmost changeset of a push rather that oldest (2017-09-05)
• link to code coverage diff viewer
• show commits contained between commits
• allow hidding commits contained within each push

The most important number, in my opinion, is the net-new-lines covered: Please make it bigger!

eg "New lines coverage change: 40.0%"

CC: @marco-c

We currently only add the summary for the tipmost changeset of a push.

We would like to do the following instead:

• Once we know that a push has coverage data do a parallel fetch for every changeset in push
  • except for merges which should not be shown anyway
• Show a summary like this (more compact & to be discussed):

New lines coverage change: 100% - Added lines: 1 / Covered lines: 1

This changeset [1] shows deleted files. We should perhaps show them as folded by default.
Issue #68 will have the state structure required to support this.

[1] https://firefox-code-coverage.herokuapp.com/#/changeset/a1b15fe9224ebd2aa5a0e398b31383f3e938ab87

if I open a link from the main page and go back in using the left arrow, then I need to wait 10 seconds to get the main page...

This is due that each view was designed as different apps with their own separate state.

Issue #68 will contain the state structure changes to support this.