Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Add it tests for main.rs and test coverage metrics #28

Merged
merged 5 commits into from
Apr 17, 2019

Conversation

javierpedreira
Copy link
Contributor

No description provided.

@javierpedreira javierpedreira changed the title Test coverage Add it tests for main.rs and test coverage metrics Apr 12, 2019
@javierpedreira javierpedreira force-pushed the test-coverage branch 2 times, most recently from 49cd66a to 52093e0 Compare April 12, 2019 08:31
@codecov-io
Copy link

codecov-io commented Apr 12, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@d5631b0). Click here to learn what that means.
The diff coverage is 95.08%.

@@           Coverage Diff            @@
##             master     #28   +/-   ##
========================================
  Coverage          ?   69.9%           
========================================
  Files             ?       3           
  Lines             ?     216           
  Branches          ?       0           
========================================
  Hits              ?     151           
  Misses            ?      65           
  Partials          ?       0

Copy link
Contributor

@nathankleyn nathankleyn left a comment

Choose a reason for hiding this comment

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

Couple of questions here! On the whole, nice work! 👏 👍

.travis.yml Outdated
@@ -1,13 +1,24 @@
sudo: required
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the sudo? I thought this opted into the older version of the Travis container.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah tha'd be because I picked and old tutorial? I'll try removing it

cache:
- cargo
- directories:
- "${HOME}/kcov/"
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't override the usual directories does it? Is it additive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we spoke, it is an arbitrary dir cache for kcov

@@ -8,6 +13,13 @@ addons:
packages:
# For building MUSL static builds on Linux.
- musl-tools
- cmake
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these new packages all for cargo-travis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes they are.

}

#[test]
#[should_panic]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice work! These annotations are awesome and never get used enough by people! 👏

fn get_repo_with_file_in_root_folder() {
let mut srv = test_server();

let query = format!("/repo/{}?file={}&reference={}", "", "README.md", "heads/master");
Copy link
Contributor

Choose a reason for hiding this comment

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

These are all interpolating static strings into another static string — best to just have one static string in thef first place?

Suggested change
let query = format!("/repo/{}?file={}&reference={}", "", "README.md", "heads/master");
let query = "/repo/?file=README.md&reference=heads/master";

Copy link
Contributor

Choose a reason for hiding this comment

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

Same for all the other format! calls in here.

- Added a badge, pointing to this branch.
- Unit tests for valid//invalid parameters for run_server
- Integration tests with a TestServer for get_repo
Copy link
Contributor

@nathankleyn nathankleyn left a comment

Choose a reason for hiding this comment

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

Nice work mate! Amazing job! 👏👏👏

@nathankleyn nathankleyn merged commit a1523f9 into master Apr 17, 2019
@nathankleyn nathankleyn deleted the test-coverage branch April 17, 2019 06:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants