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

X11vnc sys tests #4

Merged
merged 15 commits into from
Jun 2, 2022
Merged

X11vnc sys tests #4

merged 15 commits into from
Jun 2, 2022

Conversation

goodboy
Copy link
Contributor

@goodboy goodboy commented May 24, 2022

As promised a small suite that demonstrates both #1, #2.

Test suite results for me are this on linux:

tests/test_x11vnc.py::test_basic_connection_maybe_auth[password=None-force_vid=None] FAILED                                                             [ 25%]
tests/test_x11vnc.py::test_basic_connection_maybe_auth[password=None-force_vid=rgba] PASSED                                                             [ 50%]
tests/test_x11vnc.py::test_basic_connection_maybe_auth[password=doggy-force_vid=None] FAILED                                                            [ 75%]
tests/test_x11vnc.py::test_basic_connection_maybe_auth[password=doggy-force_vid=rgba] PASSED                                                            [100%]

Note that this obviously builds on PR #3 so only this test module file should be reviewed here.

Further TODOs

assert ret


# TODO: parametrization with a passwd for
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh wait i already did this.

'localhost',
port=port,
force_video_mode=force_vid,
# TODO: show this is broken
Copy link
Contributor Author

Choose a reason for hiding this comment

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

already done.

@goodboy
Copy link
Contributor Author

goodboy commented May 25, 2022

@barneygale any thoughts on this suite?

@barneygale
Copy link
Owner

Firstly, thanks tons, these looks brilliant.

I'm strapped for time right now but will have a look at the weekend!

@goodboy
Copy link
Contributor Author

goodboy commented May 26, 2022

@barneygale no rush amigo, I just want to make sure we're starting on common ground for integration with existing server impls 🏄🏼

This way I don't have to prove my problems in issues I can just have you see them in person 😂

Also on another note, I think it might be pretty slick to check out pytest-docker-tools for running some end to end tests where we just launch a super simple something or other inside vnc and make sure we can send clicks and commands and stuff?

@goodboy
Copy link
Contributor Author

goodboy commented May 31, 2022

@barneygale added an actions run that takes care of everything to get your existing test suite and these new tests running.

Feel free to to make suggestions on config changes, I just picked what I'm currently using as defaults (eg. py3.10).

asyncvnc.py Outdated
mode_data[2] &= 1 # set big endian flag to 0 or 1
mode_data[3] &= 1 # set true colour flag to 0 or 1
mode = video_modes.get(bytes(mode_data))

# # allow user to force a ``mode: str`` if they know it works.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will drop the history that brought in this patch set if you're happy with the test suite @barneygale.

So in other words I'll drop the history from #3 before you merge this.

@goodboy
Copy link
Contributor Author

goodboy commented May 31, 2022

There dropped the history from #3 so this is only the tests and CI now.

@goodboy
Copy link
Contributor Author

goodboy commented May 31, 2022

First run on our fork is here:
https://github.com/pikers/asyncvnc/runs/6676536513

@goodboy
Copy link
Contributor Author

goodboy commented May 31, 2022

CI run testing here:
pikers#2

I think we need to install an X framebuffer emulator as well.
Will come back to this later.

goodboy added 12 commits May 31, 2022 17:13
This is a small starter test suite which verifies basic connection and
auth with a local `x11vnc` server run in a sub-process. Obviously the
`x11vnc` program need to be installed locally for this suite to run.

At the time of writing this test suite exemplifies the following issues:
- barneygale#1
- barneygale#2

Further work to put this into CI is not included here but is highly
recommended ;)
Installs `asyncvnc`, `x11vnc` and runs the test suite on Python 3.10

Resolves barneygale#5
Use `shell=True` to `Popen`, wait for specific stderr output pattern
instead or arbitrary sleep.
@goodboy goodboy force-pushed the x11vnc_sys_tests branch 2 times, most recently from 7411a97 to b682e3d Compare June 1, 2022 00:02
@goodboy
Copy link
Contributor Author

goodboy commented Jun 1, 2022

@barneygale well it took a little more tweaking then I anticipated 😂 but there's a clean run here:
https://github.com/pikers/asyncvnc/runs/6680628585?check_suite_focus=true

Gonna add a badge to the readme and I think this should be good to land 🏄🏼

@goodboy
Copy link
Contributor Author

goodboy commented Jun 1, 2022

There, badge added and I think you can just merge this and get free CI which should catch any regressions on #2 and #1 💥

@barneygale
Copy link
Owner

Thanks so much!

@barneygale barneygale merged commit 8254475 into barneygale:main Jun 2, 2022
goodboy added a commit to pikers/piker that referenced this pull request Jun 2, 2022
Now that we have working client auth thanks to:
barneygale/asyncvnc#4 and related issue,
we can use a pw for the vnc server, though we should eventually
auto-generate a random one from a docker super obviously.

Add logic to the data reset hack loop to do a connection reset after
2 failed/timeout attempts at the regular data reset. We need to also add
this logic around reconnectionn events that are due to the host
network connection: aka roaming that's faster then timing logic
builtin to the gateway.
goodboy added a commit to pikers/piker that referenced this pull request Jun 3, 2022
Now that we have working client auth thanks to:
barneygale/asyncvnc#4 and related issue,
we can use a pw for the vnc server, though we should eventually
auto-generate a random one from a docker super obviously.

Add logic to the data reset hack loop to do a connection reset after
2 failed/timeout attempts at the regular data reset. We need to also add
this logic around reconnectionn events that are due to the host
network connection: aka roaming that's faster then timing logic
builtin to the gateway.
goodboy added a commit to pikers/piker that referenced this pull request Jun 5, 2022
Now that we have working client auth thanks to:
barneygale/asyncvnc#4 and related issue,
we can use a pw for the vnc server, though we should eventually
auto-generate a random one from a docker super obviously.

Add logic to the data reset hack loop to do a connection reset after
2 failed/timeout attempts at the regular data reset. We need to also add
this logic around reconnectionn events that are due to the host
network connection: aka roaming that's faster then timing logic
builtin to the gateway.
@goodboy goodboy deleted the x11vnc_sys_tests branch August 22, 2022 15:52
goodboy added a commit to pikers/piker that referenced this pull request Aug 22, 2022
We helped drive a bunch of fixes in
barneygale/asyncvnc#4

This pins to our forked but matched `main` branch to get those fixes
until such a time as upstream makes another release.
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

2 participants