Skip to content

Travis CI: Add pytest --doctest-modules file_transfer_protocol - #1044

Closed
cclauss wants to merge 4 commits into
masterfrom
Fix-ftp-to-allow-doctests
Closed

Travis CI: Add pytest --doctest-modules file_transfer_protocol#1044
cclauss wants to merge 4 commits into
masterfrom
Fix-ftp-to-allow-doctests

Conversation

@cclauss

@cclausscclauss commented Jul 19, 2019

Copy link
Copy Markdown
Member

Fixes#983 The two Python files in the file_transfer_protocol directory currently fail the Travis CI pytests. This pull request makes the changes necessary for Travis CI to run pytest on these files without hanging.

@cclausscclauss changed the title if __name == '__main__': to allow doctestsif __name__ == '__main__': to allow doctestsJul 19, 2019
@cclauss
cclaussforce-pushed the Fix-ftp-to-allow-doctests branch from b106097 to bb458f6CompareJuly 19, 2019 09:02
@cclausscclauss added the awaiting reviews This PR is ready to be reviewed label Jul 19, 2019
@cclausscclauss changed the title if __name__ == '__main__': to allow doctestsTravis CI: Add pytest --doctest-modules file_transfer_protocolJul 20, 2019

@AlexDvorakAlexDvorak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In file_transfer_protocol/ftp_client_server.py the client part of the code never runs because there is no exit condition (which is fine) that I see in the server's loop. Also, there is no way to run both the client or the server, they both have to be run.

conn.close()


# client side server

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't the client side server be split into a separate file?

@AlexDvorakAlexDvorakJul 20, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

given the loop above, this should be in a separate file


with open('received_file', 'wb') as f:
print('file opened')
while True:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This loop never exits, thus the client side server below never runs.

@cclauss

Copy link
Copy Markdown
MemberAuthor

@AlexDvorak I agree. This PR was just trying to get the pytests to run across the entire repo.

Please create a separate pull request that deals with all of the issues that you have raised. I will gladly review that PR when it passes our Travis CI tests and then close this PR in favor of yours

@cclauss

Copy link
Copy Markdown
MemberAuthor

@AlexDvorak Your thoughts on progressing this in a separate PR?

@AlexDvorak

AlexDvorak commented Jul 30, 2019 via email

Copy link
Copy Markdown
Contributor

@cclausscclauss closed this Aug 9, 2019
@dhruvmanila
dhruvmanila deleted the Fix-ftp-to-allow-doctests branch October 1, 2022 06:01
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting reviewsThis PR is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pytest --doctest-modules file_transfer_protocol fails

2 participants

@cclauss@AlexDvorak