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

do not start fullscreen in nonqubes #1134

Merged
merged 1 commit into from Aug 12, 2020
Merged

do not start fullscreen in nonqubes #1134

merged 1 commit into from Aug 12, 2020

Conversation

sssoleileraaa
Copy link
Contributor

Description

Qubick off-sprint PR to no longer start the client in fullscreen.

Test Plan

@eloquence
Copy link
Member

Works as intended in my Ubuntu env, but when I attempt to run the client from this branch in sd-app via python -m securedrop_client, the window never opens after the login screen (tried in offline mode). No error in logs/stdout/stderr, and running from an earlier commit works.

@sssoleileraaa
Copy link
Contributor Author

whoops, left out one critical line, fixed!

@eloquence
Copy link
Member

eloquence commented Aug 5, 2020

LGTM in both environments now, though looks like we have two legit test failures:

tests/gui/test_main.py::test_show_main_window_without_username FAILED [ 15%]
tests/gui/test_main.py::test_show_main_window FAILED [ 70%]

and a drop in coverage

securedrop_client/gui/main.py 94 5 95% 105-111

@sssoleileraaa
Copy link
Contributor Author

thanks for the review! yes, showing the main windows needs coverage for both qubes and non-qubes now. this has been updated so we should see ci go green

@eloquence eloquence added this to Ready for Review in SecureDrop Team Board Aug 6, 2020
@emkll emkll moved this from Ready for Review to Under Review in SecureDrop Team Board Aug 12, 2020
@emkll
Copy link
Contributor

emkll commented Aug 12, 2020

Rebased on latest main

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

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

Thanks @creviera , LGTM based on test plan:

  • Start the client in non-qubes and verify that the client shows up max-window instead of fullscreen
  • Start the client in qubes and verify that the client shows up max-window and that Part of the client window is hidden when resized #1109 is not reintroduced
  • Test coverage is 100%

@sssoleileraaa sssoleileraaa merged commit 2545d16 into main Aug 12, 2020
SecureDrop Team Board automation moved this from Under Review to Done Aug 12, 2020
@sssoleileraaa sssoleileraaa deleted the no-fullscreen branch August 12, 2020 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants