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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanups of testbed probe usage to get some missing coverage. #1830

Merged
merged 6 commits into from
Mar 28, 2023

Conversation

freakboy3742
Copy link
Member

@freakboy3742 freakboy3742 commented Mar 27, 2023

The recent audit of Divider missed some coverage because a probe was being used instead of a native getter on the implementation. The audit of Box somehow completely missed committing the entire GTK and Winforms implementations.

This PR:

  • updates the tests to use the native widget getter rather than a probe property when a widget getter exists.
  • Adds the Winforms and GTK implementation of the Box probe that was somehow missed in the commit of [widget audit] toga.Box #1820.
  • Reworks the way GTK applies style to ensure that the native widget will always have the required CSS style attributes.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@mhsmith
Copy link
Member

mhsmith commented Mar 27, 2023

I think the implicit skip approach causes too much risk of tests (and therefore bugs) being missed, so I've changed it to explicitly skip tests for widgets which are known to be unimplemented on the current platform. I prefer this over listing the platforms which *are" implemented, because it's safer to default to running too many tests rather than too few.

@freakboy3742
Copy link
Member Author

Good call - explicit skipping will definitely be less prone to error (and won't hide misspelled probe/missing problems).

@freakboy3742 freakboy3742 merged commit 466f8b1 into beeware:main Mar 28, 2023
@freakboy3742 freakboy3742 deleted the divider-coverage branch March 28, 2023 01:37
@mhsmith mhsmith mentioned this pull request Apr 14, 2023
4 tasks
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.

2 participants