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

fix: tests and styles #74

Merged
merged 1 commit into from
Aug 20, 2022
Merged

fix: tests and styles #74

merged 1 commit into from
Aug 20, 2022

Conversation

bluet
Copy link
Owner

@bluet bluet commented May 16, 2022

port to python >=3.8

deprecate <3.8

@bluet bluet requested a review from afunTW May 16, 2022 04:17
@sonarcloud
Copy link

sonarcloud bot commented May 16, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@@ -61,6 +62,8 @@ async def negotiate(self, **kwargs):
await self._proxy.send(struct.pack('3B', 5, 1, 0))
resp = await self._proxy.recv(2)

if not isinstance(resp, (bytes, str)):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you encountered any issues here?
Because the response comes from the python build-in asyncio.StreamReader, it should return the bytes object.

@@ -92,6 +95,9 @@ async def negotiate(self, **kwargs):

await self._proxy.send(struct.pack('>2BH5B', 4, 1, port, *bip, 0))
resp = await self._proxy.recv(8)
if isinstance(resp, asyncio.Future):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you encountered any issues here?
Because the response comes from asyncio and it should only possibly be the Future object.

@@ -58,6 +58,8 @@ def get_all_ip(page):

def get_status_code(resp, start=9, stop=12):
try:
if not isinstance(resp, (bytes, str)):
Copy link
Collaborator

Choose a reason for hiding this comment

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

From the viewpoint to check the external argument is acceptable, but the resp in this module should be all bytes object, and what I would like to know is there any edge case happened?

@@ -44,13 +44,13 @@ def test_base_attrs(proxy, ngtr, check_anon_lvl, use_full_path):
(
'SOCKS5',
80,
future_iter(b'\x05\x00', b'\x05\x00\x00\x01\xc0\xa8\x00\x18\xce\xdf'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason to remove this? And so does all operation in tests folder

@bluet bluet mentioned this pull request Aug 9, 2022
Closed
@bluet
Copy link
Owner Author

bluet commented Aug 9, 2022

@afunTW I was just trying to fetch the changes from @a5r0n 's branch hahaha

hi @a5r0n would you be ok to discuss here?

@a5r0n
Copy link

a5r0n commented Aug 14, 2022

@bluet i'm not really in the context, it's been a while, but let's give it a try

@afunTW
Copy link
Collaborator

afunTW commented Aug 20, 2022

@a5r0n I revise the CI flow in #79 and #80, would you like to rebase and trigger the latest CI again?

I checkout to this branch and got 53 passed, 56 warning, no failed in Python 3.10 pytest

@afunTW
Copy link
Collaborator

afunTW commented Aug 20, 2022

Since the PR mainly revise the asyncio break change https://docs.python.org/3/library/asyncio-queue.html#asyncio.Queue, and has been test. Will merge for further maintainese.

@afunTW afunTW merged commit e3dc824 into bluet:master Aug 20, 2022
@bluet
Copy link
Owner Author

bluet commented Aug 20, 2022

Awesome 👍

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