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

[9485] Skip CModuleSendmsgTests.test_shortsend #1038

Merged
merged 3 commits into from
Jul 11, 2018
Merged

[9485] Skip CModuleSendmsgTests.test_shortsend #1038

merged 3 commits into from
Jul 11, 2018

Conversation

twm
Copy link
Contributor

@twm twm commented Jul 10, 2018

As discussed on the mailing list. I opted for a .misc newsfile as I don't think this is likely to interest anyone reading the changelog.

Contributor Checklist:

twm added 2 commits July 9, 2018 21:54
The check on Travis seems to think that the ticket number is a comment.
It is failing like this:

src/twisted/python/test/test_sendmsg.py (80.0%):
    413: W9401: (Used for checking comment format issues.), : Comments should begin with one whitespace

Attempt to work around this by removing the # character.
@twm twm requested a review from a team July 10, 2018 05:55
Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup.

.misc fragment is perfect :)

Rather than keeping code which is never run ... maybe we should just delete it.

We can copy the test in the ticket details for 9452 to make it easier to find it.

I don't see the point of keeping a test which is always skipped.
I feel that we are abusing the test skip functionality... and we turn it into a .todo thing.
Is ok to skip a test on Windows, as long as it runs on Linux.

At some point, I have cleaned/removed the .todo tests.
The .todo tests were code with time became code which is not maintained.


We can merge it as is, but I prefer to just remove the test and have a clean code.

Thanks!

@codecov
Copy link

codecov bot commented Jul 11, 2018

Codecov Report

Merging #1038 into trunk will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##            trunk    #1038      +/-   ##
==========================================
+ Coverage   91.85%   91.89%   +0.04%     
==========================================
  Files         844      844              
  Lines      150605   150606       +1     
  Branches    13142    13142              
==========================================
+ Hits       138332   138400      +68     
+ Misses      10176    10114      -62     
+ Partials     2097     2092       -5

@twm twm merged commit 8191f87 into trunk Jul 11, 2018
@twm twm deleted the 9485-shortsend branch July 11, 2018 05:53
@glyph
Copy link
Member

glyph commented Jul 11, 2018

Thank you @twm, thank you @adiroiban !

@twm twm mentioned this pull request Jul 17, 2018
4 tasks
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.

None yet

3 participants