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.

Sign up for GitHub

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

Jump to bottom

Python3 port for test_networkgroup.py, test_protocol.py #1792

Closed
wants to merge 1 commit into from
Closed

Python3 port for test_networkgroup.py, test_protocol.py #1792

wants to merge 1 commit into from

Conversation

kdcis
Copy link
Contributor

@kdcis kdcis commented Jul 27, 2021 •
edited
Loading

Fix Imports
Contains changes from #1786 for protocol.py as it needed to run test_networkgroup
Handled bytes to string comparison/concatenation cases
Python 2/3 compatibility

PeterSurda
PeterSurda requested changes Jul 28, 2021
View reviewed changes
@kdcis kdcis force-pushed the v0.6-test-networkgroup branch from 8f0b2b4 to 1e9ed53 Compare July 28, 2021 13:38
@kdcis kdcis mentioned this pull request Jul 28, 2021
Closed
@kdcis kdcis force-pushed the v0.6-test-networkgroup branch from a64bc0b to 116e002 Compare July 28, 2021 14:16
@kdcis kdcis changed the title Python3 port for test_networkgroup.py Python3 port for test_networkgroup.py, test_protocol.py Jul 29, 2021
@kdcis kdcis closed this Jul 29, 2021
@kdcis kdcis reopened this Jul 29, 2021
@kdcis kdcis requested a review from PeterSurda August 2, 2021 08:57
@kdcis kdcis force-pushed the v0.6-test-networkgroup branch from 4e1b3c9 to 038bc2e Compare August 3, 2021 10:03
PeterSurda
PeterSurda requested changes Aug 3, 2021
View reviewed changes
g1itch
g1itch reviewed Aug 3, 2021
View reviewed changes
@kdcis kdcis force-pushed the v0.6-test-networkgroup branch 2 times, most recently from f25c7a4 to 316e6ef Compare August 3, 2021 11:31
g1itch
g1itch requested changes Aug 3, 2021
View reviewed changes
Copy link
Collaborator

@g1itch g1itch left a comment

Choose a reason for hiding this comment

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

You've lost the logic of the tests refactoring. network group test goes to test_protocol

@kdcis kdcis force-pushed the v0.6-test-networkgroup branch from 316e6ef to 58364ed Compare August 3, 2021 11:37
g1itch
g1itch reviewed Aug 3, 2021
View reviewed changes
g1itch
g1itch reviewed Aug 3, 2021
View reviewed changes
@kdcis kdcis marked this pull request as draft August 3, 2021 11:40
@kdcis kdcis force-pushed the v0.6-test-networkgroup branch from 58364ed to 1adeeb9 Compare August 3, 2021 11:55
g1itch
g1itch reviewed Aug 3, 2021
View reviewed changes
g1itch
g1itch reviewed Aug 3, 2021
View reviewed changes
@kdcis kdcis force-pushed the v0.6-test-networkgroup branch from 3fc4df3 to 41c8106 Compare August 3, 2021 12:17
g1itch
g1itch approved these changes Aug 3, 2021
View reviewed changes
Copy link
Collaborator

@g1itch g1itch left a comment

Choose a reason for hiding this comment

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

OK. Now you need a Peter's review. I see this as a good material for squashing (;

@kdcis kdcis force-pushed the v0.6-test-networkgroup branch from 41c8106 to 3ce3cea Compare August 3, 2021 13:30
@kdcis kdcis marked this pull request as ready for review August 3, 2021 13:35
@kdcis kdcis requested a review from PeterSurda August 3, 2021 13:36
@g1itch
Copy link
Collaborator

g1itch commented Aug 4, 2021

Next time, please don't squash into one commit. Keep test changes separate from the code changes and write proper commit messages: what you've found in test results, what was changed to fix the issue. Your commit message is sick and not informative.

The proper message might be like this:

Enabled test_protocol and test_networkgroup on python3:
    
all the tests are in test_protocol.TestProtocol,
added test_checkIPv4Address(), test_checkIPv6Address(),
test_network_group() is extended with more samples,
separated test_check_local_socks() - this one still not works on python3.
    
That all essentially is by changing the literals in the corresponding
functions in the protocol module to bytes. Also b'\x01\x02'[0] == 0x1 in py3.

@g1itch
Copy link
Collaborator

g1itch commented Aug 11, 2021

Bad signature as well here

@kdcis kdcis force-pushed the v0.6-test-networkgroup branch from 3ce3cea to bde6877 Compare August 12, 2021 07:29
@kdcis
Copy link
Contributor Author

kdcis commented Aug 12, 2021

Bad signature as well here

Updated verified commit.

@kdcis kdcis force-pushed the v0.6-test-networkgroup branch from bde6877 to b6d7fbe Compare August 12, 2021 07:36
@g1itch
Copy link
Collaborator

g1itch commented Aug 12, 2021

Bad signature as well here

Updated verified commit.

Hmm, it looks like you've changed a GPG key...

@kdcis
Copy link
Contributor Author

kdcis commented Aug 12, 2021

Bad signature as well here

Updated verified commit.

Hmm, it looks like you've changed a GPG key...

Yes.

g1itch
g1itch requested changes Aug 13, 2021
View reviewed changes
Copy link
Collaborator

@g1itch g1itch left a comment

Choose a reason for hiding this comment

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

Please rebase and change the commit message.

@kdcis kdcis force-pushed the v0.6-test-networkgroup branch from b6d7fbe to 9bb8042 Compare August 13, 2021 11:48
@kdcis
Replaced string with bytes literal, refactored & formatted the code
287abf1 
few changes

added tearDown() & fixed code quality

fixed import errors & code quality

formatted, refactored & cleaned code

fixed code quality

fixed Pylint E401

fixed imports

merged glitch's changes

added tests for protocol

refactored test_networkgroup

reverted import changes

fixed code quality

removed test_networkgroup module & fixed code quality
@kdcis kdcis force-pushed the v0.6-test-networkgroup branch from 9bb8042 to 287abf1 Compare August 13, 2021 15:25
g1itch added a commit to g1itch/PyBitmessage that referenced this pull request Aug 13, 2021
@g1itch
Separate the test using protocol.checkSocksIP() and skip it on python3
f770635 
Closes: Bitmessage#1792
g1itch added a commit to g1itch/PyBitmessage that referenced this pull request Aug 13, 2021
@g1itch
Separate the test using protocol.checkSocksIP() and skip it on python3
08b832e 
Closes: Bitmessage#1792
g1itch added a commit to g1itch/PyBitmessage that referenced this pull request Aug 14, 2021
@g1itch
Separate the test using protocol.checkSocksIP() and skip it on python3
b339e96 
Closes: Bitmessage#1792
g1itch added a commit to g1itch/PyBitmessage that referenced this pull request Aug 16, 2021
@g1itch
Separate the test using protocol.checkSocksIP() and skip it on python3
1d7761e 
Closes: Bitmessage#1792
@g1itch g1itch closed this in 3bd3f8e Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@g1itch g1itch g1itch requested changes

@PeterSurda PeterSurda PeterSurda approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kdcis @g1itch @PeterSurda