Enable container logging and add large test logic and documentation #386

deeplow · 2023-04-06T11:45:56Z

Adds a large pool of document that can and should be used prior to a release to understand effects of the new release over a real-world scenario.

Documents are stored in an external git LFS repo under tests/test_docs_large and currently it's about 11K documents gathered from multiple PDF readers and office suite's test sets.

Documentation on how to run the tests is under docs/developer/TESTING.md

Fixes #319

sssoleileraaa · 2023-05-04T16:26:26Z

Closes #352

eloquence · 2023-05-04T17:09:57Z

This is great! As discussed at today's meeting, I suggest we add steps for capturing this data on an ongoing basis (both performance and success/failure rates) so we can compare release-to-release.

apyrgio

I made a first pass of the code for this feature. Aside for some changes in the way we handle process output, due to its sensitive nature, I don't have much else to comment on. I'm eager to see this merged!

container/dangerzone.py

dangerzone/isolation_provider/container.py

container/dangerzone.py

dangerzone/isolation_provider/container.py

RELEASE.md

tests/conftest.py

tests/test_large_set.py

apyrgio · 2023-06-13T15:31:57Z

Reminder, this PR will effectively close #319, so let's update the commit messages as well.

deeplow · 2023-07-10T09:38:29Z

I have rebased the PR from main. So it now includes all the Qubes-integration stuff.

@apyrgio I also considered following your suggestion of sending data via stdout as soon as it's generated. However, I remembered why I had chosen not to do it: it would affect docker as well.

When you send data via stdout on docker, we should revert this and make use of both stdout and stderr.

apyrgio · 2023-07-12T16:24:00Z

Question, how does this PR affect our QA? Should we add a section where we ask devs to run the tests in a specific environment and capture the results?

dangerzone/isolation_provider/container.py

apyrgio · 2023-07-26T08:56:26Z

We are thiiis close to having it ready for 0.4.2, but unfortunately we won't make it. We have to push this again to 0.5.0.

dangerzone/isolation_provider/qubes.py

Makefile

dangerzone/conversion/doc_to_pixels.py

apyrgio · 2023-08-21T13:56:24Z

Besides the above comments, I have two more general ones:

I noticed that when we run ./dev_scripts/dangerzone-cli, we always see the command output. I think it would be better do so with a -v switch, unless there's an error. Since it's not something that end-users see, I don't consider this a blocker.
I suggest squashing some of the latter commits with earlier ones, because it's a bit weird reviewing some code, and a few commits later slightly changing it.

Other than that, I think we are very close to merging it.

deeplow · 2023-08-22T10:06:31Z

I noticed that when we run ./dev_scripts/dangerzone-cli, we always see the command output. I think it would be better do so with a -v switch, unless there's an error. Since it's not something that end-users see, I don't consider this a blocker.

OK. Leaving this as-is now. It would require passing the -v flag value down quite a few calls in the stack.

I suggest squashing some of the latter commits with earlier ones, because it's a bit weird reviewing some code, and a few commits later slightly changing it.

Sorry about that. I've done already a lot of that in earlier commits because the whole PR changed based on feedback and main changing already for too releases. So this is a 'best effort' but probably won't be perfect in the end. So after approval, I'll do some final squashing.

apyrgio · 2023-08-22T10:46:26Z

I've resolved all the conversations. Feel free to merge!

Use qrexec stdout to send conversion data (pixels) and stderr to send conversion progress at the end of the conversion. This happens regardless of whether or not the conversion is in developer mode or not. It's the client that decides if it reads the debug data from stderr or not. In this case, it only reads it if developer mode is enabled.

Ensure a server cannon keep the client hannging if more data than necessary is sent. This applies to container and the Qubes implmentation.

Store the conversion log to a file (captured-output.txt) in the container and when in development mode, have its output displayed on the terminal output.

Certain characters may be abused. Particularly ANSI escape codes. Solution inspired by Qubes OS's hardening of ther RPC mechanism [1]: > Terminal control characters are a security issue, which in worst case > amount to arbitrary command execution. In the simplest case this > requires two often found codes: terminal title setting (which puts > arbitrary string in the window title) and title repo reporting (which > puts that string on the shell's standard input. [sic] > > -- qvm-run.rst [2] [1]: QubesOS/qubes-core-admin-linux@e005836 [2]: https://github.com/QubesOS/qubes-core-admin-client/blob/c70da44702109b45d960c7de170ab8c2820b7167/doc/manpages/qvm-run.rst?plain=1#L126

Add GPG-styled "armor" around conversion logs -----CONVERSION LOG START----- Creator: Writer Producer: LibreOffice 6.4 [...] -----CONVERSION LOG END-----

Adds a large pool of document that can and should be used prior to a release to understand effects of the new release over a real-world scenario. Documents are stored in an external git LFS repo under `tests/test_docs_large` and currently it's about 11K documents gathered from multiple PDF readers and office suite's test sets. Documentation on how to run the tests is under `docs/developer/TESTING.md`

Log commands so we can trace back which errors / outputs are from each command.

In the junitxml this prefix would look ugly ("&gt&gt&gt") because it has to escape any non-xml tags.

Reporting script now parses JunitXML instead of a series of ".container_log" files. The script in in changed submodule. Additionally it makes failed tests actually fail so that this is recorded in the JunitXML report.

Some tests are expected to fail. To avoid having potentially thousands of tracebacks of the failed docs at the end, we're deactivating that reporting.

deeplow force-pushed the 334-large-test branch from fde4883 to eb1aa54 Compare April 6, 2023 11:51

eloquence changed the title ~~Enble container logging and add large test logic and documentation~~ Enable container logging and add large test logic and documentation Apr 18, 2023

apyrgio requested changes Jun 12, 2023

View reviewed changes

apyrgio added this to the 0.4.2 milestone Jun 26, 2023

deeplow force-pushed the 334-large-test branch from eb1aa54 to 00e40ab Compare July 10, 2023 08:20

apyrgio reviewed Jul 12, 2023

View reviewed changes

dangerzone/isolation_provider/container.py Outdated Show resolved Hide resolved

deeplow force-pushed the 334-large-test branch from cf8d9f9 to f214a04 Compare July 24, 2023 14:29

apyrgio modified the milestones: 0.4.2, 0.5.0 Jul 26, 2023

deeplow force-pushed the 334-large-test branch from 35daab9 to e9882bf Compare August 8, 2023 11:12

deeplow marked this pull request as ready for review August 8, 2023 14:26

deeplow mentioned this pull request Jun 21, 2023

Qubes: Error handling #430

Closed

10 tasks

apyrgio requested changes Aug 21, 2023

View reviewed changes

dangerzone/isolation_provider/qubes.py Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

dangerzone/conversion/doc_to_pixels.py Outdated Show resolved Hide resolved

deeplow force-pushed the 334-large-test branch from a0dbf9b to 378ec1c Compare August 22, 2023 09:43

apyrgio approved these changes Aug 22, 2023

View reviewed changes

deeplow force-pushed the 334-large-test branch from c56c7dd to fb560b1 Compare August 22, 2023 14:40

deeplow added 5 commits August 22, 2023 16:11

Qubes: close qrexec stdin and stout

d6bce4d

Ensure a server cannon keep the client hannging if more data than necessary is sent. This applies to container and the Qubes implmentation.

Ignore large tests when doing "make test"

e2accc2

Containers: capture conversion logs

95cef8c

Store the conversion log to a file (captured-output.txt) in the container and when in development mode, have its output displayed on the terminal output.

deeplow added 9 commits August 22, 2023 16:11

Add "armor" around conversion log

f41cefd

Add GPG-styled "armor" around conversion logs -----CONVERSION LOG START----- Creator: Writer Producer: LibreOffice 6.4 [...] -----CONVERSION LOG END-----

Log command to debug log for traceback purposes

48b2e7b

Log commands so we can trace back which errors / outputs are from each command.

Replace container output command prefix ">>>"

eb16285

In the junitxml this prefix would look ugly ("&gt&gt&gt") because it has to escape any non-xml tags.

Adapt code so it works for reporting script

75369cf

Reporting script now parses JunitXML instead of a series of ".container_log" files. The script in in changed submodule. Additionally it makes failed tests actually fail so that this is recorded in the JunitXML report.

Add logging for second container

fa21506

Large test run: hide traceback to avoid spam

a0bcd12

Some tests are expected to fail. To avoid having potentially thousands of tracebacks of the failed docs at the end, we're deactivating that reporting.

Replace armor guards that indicate isolated output

9ec9cc5

Add tests documentation

89365b5

deeplow force-pushed the 334-large-test branch from fb560b1 to 89365b5 Compare August 22, 2023 15:14

deeplow merged commit 89365b5 into main Aug 22, 2023

deeplow deleted the 334-large-test branch August 22, 2023 15:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable container logging and add large test logic and documentation #386

Enable container logging and add large test logic and documentation #386

deeplow commented Apr 6, 2023 •

edited

Loading

sssoleileraaa commented May 4, 2023

eloquence commented May 4, 2023

apyrgio left a comment

apyrgio commented Jun 13, 2023

deeplow commented Jul 10, 2023

apyrgio commented Jul 12, 2023

apyrgio commented Jul 26, 2023

apyrgio commented Aug 21, 2023

deeplow commented Aug 22, 2023

apyrgio commented Aug 22, 2023

Enable container logging and add large test logic and documentation #386

Enable container logging and add large test logic and documentation #386

Conversation

deeplow commented Apr 6, 2023 • edited Loading

sssoleileraaa commented May 4, 2023

eloquence commented May 4, 2023

apyrgio left a comment

Choose a reason for hiding this comment

apyrgio commented Jun 13, 2023

deeplow commented Jul 10, 2023

apyrgio commented Jul 12, 2023

apyrgio commented Jul 26, 2023

apyrgio commented Aug 21, 2023

deeplow commented Aug 22, 2023

apyrgio commented Aug 22, 2023

deeplow commented Apr 6, 2023 •

edited

Loading