Skip to content

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

Closed
kdcis wants to merge 1 commit into
Bitmessage:v0.6from
kdcis:v0.6-test-networkgroup
Closed

Python3 port for test_networkgroup.py, test_protocol.py#1792
kdcis wants to merge 1 commit into
Bitmessage:v0.6from
kdcis:v0.6-test-networkgroup

Conversation

@kdcis
Copy link
Copy Markdown
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
Comment thread src/highlevelcrypto.py Outdated
Comment thread src/protocol.py Outdated
Comment thread src/protocol.py Outdated
Comment thread src/protocol.py Outdated
Comment thread src/protocol.py Outdated
Comment thread src/tests/common.py Outdated
Comment thread src/tests/test_networkgroup.py Outdated
Comment thread src/tests/test_networkgroup.py Outdated
Comment thread src/tests/test_networkgroup.py Outdated
Comment thread src/tests/test_networkgroup.py Outdated
@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
Comment thread src/protocol.py Outdated
Comment thread src/protocol.py
Comment thread src/tests/common.py Outdated
Comment thread src/tests/test_networkgroup.py Outdated
Comment thread src/tests/test_networkgroup.py Outdated
Comment thread src/tests/test_networkgroup.py Outdated
g1itch
g1itch reviewed Aug 3, 2021
View reviewed changes
Comment thread src/tests/test_networkgroup.py Outdated
g1itch
g1itch reviewed Aug 3, 2021
View reviewed changes
Comment thread src/tests/common.py Outdated
@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
Copy Markdown
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

Comment thread src/tests/test_protocol.py
Comment thread src/tests/test_networkgroup.py Outdated
@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
Comment thread src/tests/test_blindsig.py Outdated
g1itch
g1itch reviewed Aug 3, 2021
View reviewed changes
Comment thread src/tests/common.py
@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
Comment thread src/tests/test_api.py
g1itch
g1itch reviewed Aug 3, 2021
View reviewed changes
Comment thread src/tests/test_blindsig.py Outdated
@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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