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.

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

Report descending sort option #1005

Merged

Conversation

jerinpetergeorge
Copy link
Contributor

Fix #199

@jerinpetergeorge
Copy link
Contributor Author

@nedbat Would you mind guide me to add the command line options? I am not much sure whether my commit, 0228c1a is okey or not for this.

@nedbat
Copy link
Owner

nedbat commented Jun 30, 2020

I've made you a pull request that fixes the tests: jerinpetergeorge#1

coverage/cmdline.py Outdated Show resolved Hide resolved
coverage/summary.py Outdated Show resolved Hide resolved
@nedbat
Copy link
Owner

nedbat commented Jun 30, 2020

BTW, at any time, you can say, "I've done as much as I want to do," and I'll pick it up, or we can keep working together on it.

@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2020

Codecov Report

Merging #1005 into master will increase coverage by 0.01%.
The diff coverage is 58.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1005      +/-   ##
==========================================
+ Coverage   94.02%   94.04%   +0.01%     
==========================================
  Files          86       86              
  Lines       12152    12173      +21     
  Branches     1220     1224       +4     
==========================================
+ Hits        11426    11448      +22     
+ Misses        592      591       -1     
  Partials      134      134              
Impacted Files Coverage Δ
coverage/control.py 92.45% <ø> (+0.80%) ⬆️
tests/test_cmdline.py 100.00% <ø> (ø)
coverage/summary.py 88.07% <44.44%> (-4.09%) ⬇️
coverage/cmdline.py 93.22% <100.00%> (+0.04%) ⬆️
coverage/config.py 98.43% <100.00%> (+<0.01%) ⬆️
tests/test_xml.py 100.00% <0.00%> (ø)
coverage/xmlreport.py 100.00% <0.00%> (ø)
... and 1 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 13159a9...2ec292d. Read the comment docs.

@nedbat
Copy link
Owner

nedbat commented Jul 1, 2020

Thanks! Are we good to merge?

@jerinpetergeorge
Copy link
Contributor Author

I think we are good to go. I will make another PR with a few tests that probably cover the new sort option.

@nedbat
Copy link
Owner

nedbat commented Jul 1, 2020

We can wait for tests :)

@jerinpetergeorge
Copy link
Contributor Author

Great. Let me try

@nedbat
Copy link
Owner

nedbat commented Jul 3, 2020

I'm thinking about a new release in a few days, and I'd like to get this into it. I don't mind doing the tests myself if you don't mind.

@jerinpetergeorge
Copy link
Contributor Author

I'm thinking about a new release in a few days, and I'd like to get this into it. I don't mind doing the tests myself if you don't mind.

That's so fast... Anyway, I have pushed one more commit.

BTW, I came across this piece of the test.

def test_sort_report_by_stmts(self):
    # Sort the text report by the Stmts column.
    report = self.get_summary_text(('report:sort', 'Stmts'))
    self.assert_ordering(report, "test_backward.py", "test_coverage.py", "test_api.py")

Just out of curiosity, I have changed the assert statement to something like self.assert_ordering(report, "test_coverage.py", "test_backward.py", "test_api.py") and ran the test. The tests are successfully executed.

Did I miss something here? @nedbat

@nedbat
Copy link
Owner

nedbat commented Jul 3, 2020

Wow, good catch on that broken test! I've fixed it in f28b1db, and added an assert to prevent that kind of breakage in the future.

Thanks for this, I'm merging it!

@nedbat nedbat merged commit ba7e1db into nedbat:master Jul 3, 2020
@jerinpetergeorge
Copy link
Contributor Author

Welcome 😄 I hope you can handle the rest of the test cases if any

@nedbat
Copy link
Owner

nedbat commented Jul 6, 2020

This is now released as part of coverage 5.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a way to sort the text report
3 participants