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.

Sign up for GitHub

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

Jump to bottom

Output lcov for frontend coverage #7152

Merged
merged 2 commits into from
Jul 18, 2019
Merged

Output lcov for frontend coverage #7152

merged 2 commits into from
Jul 18, 2019

Conversation

kevinlee12
Copy link
Contributor

Explanation

The following changes the output of the test coverage report to lcov. The old json report will no longer be reported. The location of the new report is ../karma_coverage_reports/lcov.info.

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The linter/Karma presubmit checks have passed.
    • These should run automatically, but if not, you can manually trigger them locally using python scripts/pre_commit_linter.py and bash scripts/run_frontend_tests.sh.
  • The PR is made from a branch that's not called "develop".
  • The PR has an appropriate "CHANGELOG: ..." label (If you are unsure of which label to add, ask the reviewers for guidance).
  • The PR follows the style guide.
  • The PR addresses the points mentioned in the codeowner checks for the files/folders changed. (See the codeowner's wiki page.)
  • The PR is assigned to an appropriate reviewer.
    • If you're a new contributor, please ask on Gitter for someone to assign a reviewer and don't tick this checkbox.
    • If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue. Do not only request the review but also add the reviewer as an assignee.

lilithxxx
lilithxxx approved these changes Jul 16, 2019
View reviewed changes
Copy link
Contributor

@lilithxxx lilithxxx left a comment

Choose a reason for hiding this comment

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

LGTM

@seanlip seanlip assigned nithusha21 and apb7 and unassigned lilithxxx Jul 16, 2019
@kevinlee12
Copy link
Contributor Author

@lilithxxx, just an fyi this doesn't send the report to Code Climate, it changes the output.

@codecov
Copy link

codecov bot commented Jul 17, 2019

Codecov Report

Merging #7152 into develop will increase coverage by 11.03%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop    #7152       +/-   ##
============================================
+ Coverage    87.01%   98.05%   +11.03%     
============================================
  Files          938      379      -559     
  Lines        85274    63193    -22081     
  Branches      2710        0     -2710     
============================================
- Hits         74204    61962    -12242     
+ Misses       11070     1231     -9839
Impacted Files Coverage Δ
core/controllers/editor.py 100% <0%> (ø) ⬆️
core/controllers/collection_editor.py 100% <0%> (ø) ⬆️
.../collection/SearchExplorationsBackendApiService.ts
.../directives/ItemSelectionInputValidationService.ts
.../domain/topic/SubtopicPageContentsObjectFactory.ts
...yer-page/services/answer-classification.service.ts
...editor-page/services/story-editor-state.service.ts
...ore/templates/dev/head/services/PromoBarService.ts
...ates/dev/head/services/PlaythroughIssuesService.ts
...irectives/OppiaShortResponseLogicProofDirective.ts
... and 552 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5cacf6...24f2602. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 17, 2019 •
edited
Loading

Codecov Report

Merging #7152 into develop will increase coverage by 0.55%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #7152      +/-   ##
===========================================
+ Coverage    87.01%   87.57%   +0.55%     
===========================================
  Files          938      938              
  Lines        85274    82886    -2388     
  Branches      2710     2307     -403     
===========================================
- Hits         74204    72585    -1619     
+ Misses       11070     9884    -1186     
- Partials         0      417     +417
Impacted Files Coverage Δ
...e/templates/dev/head/services/ValidatorsService.ts 60% <0%> (-20.44%) ⬇️
...itor-properties-services/state-property.service.ts 56.52% <0%> (-20.15%) ⬇️
...omain/question/PretestQuestionBackendApiService.ts 75% <0%> (-16.31%) ⬇️
...-page/services/state-classifier-mapping.service.ts 80% <0%> (-14.74%) ⬇️
.../head/filters/remove-duplicates-in-array.filter.ts 71.42% <0%> (-14.29%) ⬇️
.../collection/ReadOnlyCollectionBackendApiService.ts 85.71% <0%> (-14.29%) ⬇️
.../services/prediction-algorithm-registry.service.ts 77.77% <0%> (-13.89%) ⬇️
core/templates/dev/head/services/ContextService.ts 63.79% <0%> (-13.84%) ⬇️
...v/head/services/ExplorationHtmlFormatterService.ts 86.2% <0%> (-13.8%) ⬇️
...xploration/ReadOnlyExplorationBackendApiService.ts 87.17% <0%> (-12.83%) ⬇️
... and 475 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5cacf6...9b2c558. Read the comment docs.

@nithusha21
Copy link
Contributor

So codecov report that is shown in the PR thread only contains the backend coverage now?

@nithusha21
Copy link
Contributor

Also, what was 87.01%? And where co we view the frontend coverage now?

@kevinlee12
Copy link
Contributor Author

Yup, the coverage only reports the backend coverage at the moment. The number changed because we're not reporting the frontend coverage to codecov. (We're still figuring out what to do at the moment).

@kevinlee12 kevinlee12 assigned kevinlee12 and unassigned nithusha21 and apb7 Jul 17, 2019
@kevinlee12
Copy link
Contributor Author

Assigning myself for now while I await further instructions.

@seanlip
Copy link
Member

seanlip commented Jul 17, 2019

@kevinlee12 -- one question: who are you awaiting instructions from? I think this PR should be assigned to that person and the ask should be clarified.

@kevinlee12
Copy link
Contributor Author

I was awaiting a reply to that email thread - should we close this or integrate lcov with codecov?

@kevinlee12 kevinlee12 assigned apb7 and unassigned kevinlee12 Jul 18, 2019
@kevinlee12
Copy link
Contributor Author

Frontend report with lcov report: https://codecov.io/gh/oppia/oppia/tree/9b2c558e1b9f2a6d8dac00ddd097363d8b4badd6

apb7
apb7 approved these changes Jul 18, 2019
View reviewed changes
Copy link
Contributor

@apb7 apb7 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @kevinlee12!

@apb7 apb7 merged commit 25f0b5d into oppia:develop Jul 18, 2019
@kevinlee12 kevinlee12 deleted the lcov-frontend-reports branch August 25, 2019 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lilithxxx lilithxxx lilithxxx approved these changes

@apb7 apb7 apb7 approved these changes

@nithusha21 nithusha21 Awaiting requested review from nithusha21

Assignees

@nithusha21 nithusha21

@apb7 apb7

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kevinlee12 @nithusha21 @seanlip @apb7 @lilithxxx