Skip to content

Conversation

@Broderick-Westrope
Copy link
Contributor

@Broderick-Westrope Broderick-Westrope commented Aug 26, 2024

This PR seeks to add unit tests for the table component as requested in #593. The hope is that the table bubble will be as thoroughly tested as the lipgloss table.

Tests have been added to address these issues:

Tests written specifically for issues will be failing until the issue is fixed (in a separate PR). Other tests have also been added to improve the general coverage (ie. not in relation to any known issue).

I welcome questions relating to this PR. I think this is a great platform for clarifying the expected/desired behaviour of the table component.

@Broderick-Westrope Broderick-Westrope changed the title feat(table): improve table unit tests test(table): improve table unit tests Aug 26, 2024
@Broderick-Westrope Broderick-Westrope marked this pull request as ready for review August 30, 2024 03:25
@meowgorithm meowgorithm requested a review from caarlos0 September 1, 2024 12:53
@aymanbagabas aymanbagabas self-requested a review December 6, 2024 14:51
@Broderick-Westrope
Copy link
Contributor Author

Hi @aymanbagabas, nice to see some activity here. I welcome any questions or change requests you may have :)

@aymanbagabas
Copy link
Member

Hi @aymanbagabas, nice to see some activity here. I welcome any questions or change requests you may have :)

Could you please look into the tests and why they're failing?

@Broderick-Westrope
Copy link
Contributor Author

@aymanbagabas the tests are failing intentionally. I was tasked with creating tests to validate the correct output. Known bugs are currently making the tests fail. Please see the PR description for more details.

@bashbunni bashbunni modified the milestones: table, Table Jan 22, 2025
@caarlos0
Copy link
Member

cc/ @bashbunni I think you also fiddling with table, not sure if you aware of this PR

Copy link
Member

@andreynering andreynering left a comment

Choose a reason for hiding this comment

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

Hi @Broderick-Westrope,

Thank you for contributing, this looks very solid! 🚀

I added skips to two tests that were failing for now because we need the CI to pass. We can un-skip them once the issues gets fixed.

I'll merge this now and then merge this work into the v2-exp branch.

From now on, please open any new PRs targeting v2-exp. Probably a good idea to wait for #772 to be merged first, though.

@andreynering andreynering linked an issue May 16, 2025 that may be closed by this pull request
@andreynering andreynering merged commit f54a125 into charmbracelet:master May 16, 2025
11 checks passed
andreynering added a commit that referenced this pull request May 16, 2025
chore: merge `master` with #601 into `v2-exp`
@andreynering
Copy link
Member

andreynering commented May 16, 2025

This work was merged on v2-exp:

@meowgorithm
Copy link
Member

Thanks for your patience with this one @Broderick-Westrope!

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.

test: improve table tests based on reported issues

6 participants