Skip to content

Chore!: Refactor sqlmesh test output to use rich#4715

Merged
VaggelisD merged 7 commits intomainfrom
vaggelisd/test_verbose_comparison
Jun 17, 2025
Merged

Chore!: Refactor sqlmesh test output to use rich#4715
VaggelisD merged 7 commits intomainfrom
vaggelisd/test_verbose_comparison

Conversation

@VaggelisD
Copy link
Copy Markdown
Collaborator

@VaggelisD VaggelisD commented Jun 11, 2025

Before this PR:

  • Default:
before
  • Verbose
before_vv

After this PR

  • Default
after
  • Verbose (columns are split pairwise)
after_vv

Comment thread sqlmesh/core/console.py Outdated
Comment thread sqlmesh/core/console.py Outdated
Comment thread sqlmesh/core/context.py Outdated
Comment thread sqlmesh/core/test/result.py Outdated
Comment thread sqlmesh/core/test/result.py
@izeigerman
Copy link
Copy Markdown
Collaborator

Shouldn't a single -v be enough for the pairwise mode? Actually, I wonder whether we should just automatically switch to the pairwise mode if the number of columns exceeds certain threshold. Ideally, this should depend on the current width of the screen. Why even output something we know is probably unreadable?

@VaggelisD
Copy link
Copy Markdown
Collaborator Author

**Shouldn't a single -v be enough for the pairwise mode? **

Yeah, that is already the case too, both -v and -vv will cause the pairwise mode.

Actually, I wonder whether we should just automatically switch to the pairwise mode if the number of columns exceeds certain threshold. Ideally, this should depend on the current width of the screen. Why even output something we know is probably unreadable?

That is a good idea, but do we think this might cause too much output and render the default version useless? Maybe a different idea would be to break the initial table apart every N columns, e.g:

> sqlmesh test 

Data mismatch:
col_1: Expected | col_1: Actual | ... | col_n: Expected | col_n: Actual|

Could become:

> sqlmesh test 

Data mismatch (chunk 1):
col_1: Expected | col_1: Actual | ... | col_5: Expected | col_5: Actual|

Data mismatch (chunk 2)
col_6: Expected | col_6: Actual | ... | col_10: Expected | col_10: Actual|

....

What do you think?

Comment thread sqlmesh/core/console.py Outdated
@izeigerman
Copy link
Copy Markdown
Collaborator

That is a good idea, but do we think this might cause too much output and render the default version useless? Maybe a different idea would be to break the initial table apart every N columns, e.g:

I think I like that

Copy link
Copy Markdown
Collaborator

@georgesittas georgesittas left a comment

Choose a reason for hiding this comment

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

LGTM, though, I'd let Iaroslav and Sung take another look at this. Looks nice 👍

@VaggelisD VaggelisD merged commit 57b59ec into main Jun 17, 2025
25 checks passed
@VaggelisD VaggelisD deleted the vaggelisd/test_verbose_comparison branch June 17, 2025 09:47
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.

3 participants