Refactor cli tests to rely on large terminal #1253
Conversation
feluelle
requested review from
dimberman,
tatiana,
pankajastro,
pankajkoti,
kaxil and
utkarsharma2
as code owners
| @@ -99,6 +99,7 @@ jobs: | |||
| env: | |||
| AIRFLOW__CORE__XCOM_BACKEND: astro.custom_backend.astro_custom_backend.AstroCustomXcomBackend | |||
| AIRFLOW__ASTRO_SDK__STORE_DATA_LOCAL_DEV: True | |||
| TERMINAL_WIDTH: 3000 | |||
There was a problem hiding this comment.
@feluelle do you see any downsides to increasing the terminal width to this value?
There was a problem hiding this comment.
Not really. The advantage is that we can start having tests checking for explicit new lines whereas if we don't set the terminal width we need to either replace the new lines always, but then we cannot check for new lines in output, to make it consistent or check for the new lines in conjunction with the actual string/output. The question is what would be ideal terminal width be? I think there is none, hence we are setting it to a large number to avoid terminal doing terminal things.
There was a problem hiding this comment.
Sounds like a good plan! I'm happy for us to merge this once the checks are passing!
There was a problem hiding this comment.
It seems setting TERMINAL_WIDTH does not change reflect the terminal size in the tests. I can reproduce on a small local terminal :(
There was a problem hiding this comment.
I found the issue 4a9811b
Codecov ReportBase: 93.96% // Head: 94.00% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1253 +/- ##
==========================================
+ Coverage 93.96% 94.00% +0.03%
==========================================
Files 18 18
Lines 613 617 +4
Branches 69 69
==========================================
+ Hits 576 580 +4
Misses 22 22
Partials 15 15
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Instead of changing the stdout via get_stdout, we just set the TERMINAL_WIDTH to a very large number to avoid new lines. The configuration has been copied from https://github.com/tiangolo/typer/blob/master/scripts/test.sh
..as this is a 1:1 copy of rich interface
feluelle
force-pushed
the
refactor/sql-cli-cli-tests
branch
from
791b8ea
to
55d2bdc
Compare
…1298) # Description ## What is the current behavior? The highlighting was changed accidentally in the related PR. related: #1253 ## What is the new behavior? We revert the highlighting to the previous default i.e. `ReprHighlighter` which highlights links for example. ## Does this introduce a breaking change? No. ### Checklist - [ ] Created tests which fail without the change (if possible) - [ ] Extended the README / documentation, if necessary
Instead of changing the stdout via get_stdout, we just set the TERMINAL_WIDTH to a very large number to avoid new lines. The configuration has been copied from https://github.com/tiangolo/typer/blob/master/scripts/test.sh
EDIT:
I have also created tiangolo/typer#509 to fix that in typer.