Skip to content
This repository has been archived by the owner on Apr 11, 2019. It is now read-only.

[WIP] Add a directory coverage viewer. #222

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jankeromnes
Copy link
Contributor

@jankeromnes jankeromnes commented Aug 21, 2018

Fixes #176.

This is a rough prototype showing per-directory coverage data:

directory-coverage

It hijacks Routes by replacing FileViewerContainer with a new DirectoryViewerContainer component when the requested file path ends with / (I'd like to find a cleaner solution to identify directories from files), and it uses an ActiveData query to aggregate coverage by sub-file and sub-folder (I'd like to switch to the faster https://coverage.staging.moz.tools/v2/path endpoint).

TODO:

  • Find a cleaner way to differentiate between "file" and "directory" (than relying on the final /)
  • Fix code duplication between fileViewer.jsx and directoryViewer.jsx
    • e.g. by merging the two files and making a few components more generic
  • Use Bastien's new https://coverage.staging.moz.tools/v2/path endpoint
    • it should be faster & more consistent
  • Turn sub-files & sub-folders into links to make the directory coverage view browsable
  • Verify that the coverage data we see looks correct

This change is Reviewable

@jankeromnes jankeromnes added enhancement view - file viewer This is the view that allows seeing tests covering a certain line on a file. labels Aug 21, 2018
@armenzg armenzg temporarily deployed to firefox-code-coverage-pr-222 August 21, 2018 19:14 Inactive
@armenzg
Copy link
Contributor

armenzg commented Aug 21, 2018

Exciting!
I've requested a PR deployment (it should be active for 5 days since the last push).

Give me a heads up when you think this is closer to being ready to be reviewed.

@marco-c
Copy link
Collaborator

marco-c commented Aug 21, 2018

Find a cleaner way to differentiate between "file" and "directory" (than relying on the final /)

This would be for free when you do:

Use Bastien's new https://coverage.staging.moz.tools/v2/path endpoint

@jankeromnes
Copy link
Contributor Author

Thanks for the feedback!

Instead of re-implementing a full separate DirectoryViewerContainer component (with similar sub-components as FileViewerContainer), I think there is a way to make FileViewerContainer also fetch https://coverage.staging.moz.tools/v2/path, and then differentiate between file and directory in the render() method directly.

This would address the first three items in the checklist.

@armenzg armenzg temporarily deployed to firefox-code-coverage-pr-222 August 22, 2018 14:36 Inactive
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Aug 22, 2018

This new commit is still very much work-in-progress, but it makes FileViewerContainer fetch the new /v2/path endpoint, and adds a bit of UI according to the results:

coverage-side-by-side

This could allow comparing ActiveData results with our new backend (but today, I don't think they are any revisions in common between both, so instead I'm fetching /v2/path without a revision, which I believe gives me the latest available data).

EDIT: Note that I'll soon remove the pointless "raw file" view of the directory. I just kept it temporarily in order to compare results between hg.m.o / ActiveData / coverage.moz.tools (and for example, /v2/path is missing some folders/files for which it has no data. Maybe we still want to list them in our view?)

@armenzg armenzg temporarily deployed to firefox-code-coverage-pr-222 August 22, 2018 14:43 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement view - file viewer This is the view that allows seeing tests covering a certain line on a file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants