dmlc / gluon-nlp

NLP made easy

https://nlp.gluon.ai/

Apache License 2.0

2.56k stars 538 forks source link

[CI] Add Codecov and Test Logs #1349

Closed barry-jin closed 4 years ago

barry-jin commented 4 years ago

Description

Add codecov report and upload batch job logs as build artifacts.

Checklist

Essentials

[x] PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
[x] Changes are complete (i.e. I finished coding on this PR)
[ ] All changes have test coverage
[ ] Code is well-documented

Changes

[ ] Feature1, tests, (and when applicable, API doc)
[ ] Feature2, tests, (and when applicable, API doc)

Comments

If this change is a backward incompatible change, why must this change be made.
Interesting edge cases to note here

cc @dmlc/gluon-nlp-team

codecov[bot] commented 4 years ago

Codecov Report

Merging #1349 into master will increase coverage by 0.10%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1349      +/-   ##
==========================================
+ Coverage   81.65%   81.75%   +0.10%     
==========================================
  Files          52       52              
  Lines        6862     6862              
==========================================
+ Hits         5603     5610       +7     
+ Misses       1259     1252       -7

Impacted Files	Coverage Δ
src/gluonnlp/data/tokenizers/subword_nmt.py	`79.43% <0.00%> (+0.93%)`	:arrow_up:
src/gluonnlp/data/loading.py	`83.39% <0.00%> (+2.26%)`	:arrow_up:

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 203d84c...cab4c6d. Read the comment docs.

sxjscience commented 4 years ago

@szha Would you review/merge?

sxjscience commented 4 years ago

For me, I feel that we can have two artifacts: 1) the log of the unittest, 2) the complete log generated by the batch job. The advantage is that we may choose to download one of them to reduce the time for investigating the log.

leezu commented 4 years ago

Having two logs may be confusing. The problem is that the unittest can fail with cryptic errors if there is a problem in the setup phase of the batch job. People may not understand that they need to check a separate log file to figure out the error

sxjscience commented 4 years ago

@leezu The problem here is that the full log can have >400MB, which looks scary for most people. We may write a README to teach the user how to investigate batch-related problems.

leezu commented 4 years ago

The problem here is that the full log can have >400MB

That's due to misconfiguration.