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

test, fix invalid user-provided startup() method #2047

Merged
merged 6 commits into from
Jul 23, 2023

Conversation

mattip
Copy link
Contributor

@mattip mattip commented Jul 22, 2023

(EuroPython sprints)
Fixes #760

Typically users will override the App.startup() method with one of their own. For most backends, this method must create a MainWindow. This PR refactors the call to startup so that after the call, a new _verify_startup() private method will be called which, for all the GUI backends, will enforce the condition. For DocumentApp app the verification is empty.

Additionally, the "dummy" test backend was not properly chaining calls so main_loop calls create which calls startup. This was changed too to make the new feature testable.

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

core/src/toga/app.py Outdated Show resolved Hide resolved
core/src/toga/app.py Outdated Show resolved Hide resolved
Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Looks good; a couple of minor suggestions provided as diffs.

The only other problem I can see is that there's no test of the DocumentApp analog of this process. It's technically covered, but there's no validation that it's actually happening.

@mattip
Copy link
Contributor Author

mattip commented Jul 22, 2023

I added a test for the DocumentApp path, and CI is passing.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

I've made a couple of minor tweaks to the error message wording, but otherwise this looks great! Thanks for the contribution.

@freakboy3742 freakboy3742 merged commit 5874d2e into beeware:main Jul 23, 2023
39 checks passed
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.

AttributeError: 'NoneType' object has no attribute '_impl'
2 participants