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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add Set DisableRender option for testing. #364

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

PThorpe92
Copy link

@PThorpe92 PThorpe92 commented Aug 20, 2023

Flag for disabling rendering #363

This PR adds a "SET" option

set DisableRender <bool>

That does the obvious 馃槃 and keeps it from executing the ffmpeg command so just .txt output can be dealt more easily for our CI/CD testing plans over at EZA as can be seen at #147

This was my first time looking at the code base, so please let me know if I missed or overlooked something. Also please let me know if/where you would like tests added for this behavior. All I added was to the default token/parser tests, and I couldn't figure out exactly where a general test for this would go, so please do let me know and I'll add it.
Everything I have tried with Set DisableRender true and with output being .txt has worked perfectly.

Just want to say thanks for a great program, and everything you guys do is awesome 馃殌 馃殌

EDIT: Just ran into this again and figured I'd fix the conflicts.

cafkafk added a commit to eza-community/eza that referenced this pull request Sep 6, 2023
Refs: #147, charmbracelet/vhs#364
Signed-off-by: Christina S酶rensen <christina@cafkafk.com>
@mbechto
Copy link

mbechto commented Nov 28, 2023

I have a similar use case. Can we have this please @maaslalani? 馃槂

@PThorpe92
Copy link
Author

I have a similar use case. Can we have this please ? 馃槂

Assuming you have the same use case (testing the output of CLI program), You might like to know that we found diffing the txt output to be less than deterministic unfortunately 馃槖

We ended up going with trycmd and are working on a program to generate the test cases.

What kind of output are you looking to test? It would prob work for something fairly simple but eza output got complex to test this way pretty quick

@mbechto
Copy link

mbechto commented Nov 28, 2023

Yes, I can confirm that depending on the machine running the tests, the results can vary. Also, if you want to make it as reproducible as possible, I found fixating the window size in columns and rows is a good idea.

In my case, I worked around both limitations using tmux to make screen captures:

Type "tmux"
Enter
Type "tmux resize-window -x 110 -y 24"
Enter
Type "tmux bind -n C-p 'capture-pane; save-buffer -a out.txt; delete-buffer'"
Enter

In each following test case, I use the Ctrl+p screen capture shortcut bound in tmux to make a combined (therefore -a append) out.txt with all screens. This is a little wacky, but it works well enough most of the time.

I had a brief look at the tools you mentioned, but I had the impression it could not be used to test an interactive TUI, that renders a lot of screens in unicode and simulating user input. I would be interested to know how do you handle this.

EDIT: Setting the window size in pixels won't work reliably. I had problems to reproduce the results on different machines (especially GitHub runners).

@PThorpe92
Copy link
Author

Ahh yeah a TUI is a little more difficult eza is a CLI app meant to serve as a replacement for LS so it is quite a bit easier than a full blown interactive TUI..

Thank you for the tip though, one of my other projects is a TUI and I was trying to figure out how best to test the UI. Hopefully they merge this, although it's been so long now that I might have to go back and take a peek to make sure everything still works but I also don't think they have merged many commits since then either.

At one point I was going to work on building and maintaining a fork of VHS that only serves to test the output of TUI applications, when we found tryCMD for eza, I changed directions but now that I am in need of testing for a TUI I might revisit that idea.

@mbechto
Copy link

mbechto commented Nov 28, 2023

If you plan to write a test tool suitable for TUIs feel free to hit me up. I like to code during my weekly train rides and would be happy to collaborate on anything Rust.

@maaslalani
Copy link
Member

maaslalani commented Mar 15, 2024

Hey @PThorpe92, thanks for this PR, I think the concept makes sense to have but I would like to avoid increasing the surface area of the language by adding a new keyword for this feature. What do you think of simply modifying the Output to recognize an argument such as "none" or "/dev/null" or something along these lines:

Output none
Output "/dev/null" # perhaps?

Omnikron13 pushed a commit to Omnikron13/eza that referenced this pull request Mar 17, 2024
Refs: eza-community#147, charmbracelet/vhs#364
Signed-off-by: Christina S酶rensen <christina@cafkafk.com>
@PThorpe92
Copy link
Author

PThorpe92 commented Mar 21, 2024

Yeah that makes sense to me, I'll make the adjustments this weekend 馃憤

EDIT: now I remember why I didn't do that the first time. That leaves nowhere for the user to specify their output.txt file.

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