8000 Minor improvement to `qtbot` testing section by lucyleeow · Pull Request #481 · napari/docs · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Minor improvement to qtbot testing section #481

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

8000
Merged
merged 3 commits into from
Aug 29, 2024
Merged

Conversation

lucyleeow
Copy link
Collaborator

References and relevant issues

Related zulip: https://napari.zulipchat.com/#narrow/stream/212875-general/topic/qtbot.20documentation

Description

  • Expand why you need QApplication running in test
  • Recommend usage of qtbot
  • improve wording in comment

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 16, 2024
@lucyleeow
Copy link
Collaborator Author

Sorry @psobolewskiPhD I want to add some more items. Just didn't get around to it yet. I'll ping you once dine, thank you!

@lucyleeow
Copy link
Collaborator Author

@psobolewskiPhD okay I've tried to add more information but I am not knowledgeable on Qt so please let me know if anything is off!

<br/>
````

`qtbot` provides a convenient
`qtbot` also provides a convenient
[`addWidget`](https://pytest-qt.readthedocs.io/en/latest/reference.html#pytestqt.qtbot.QtBot.addWidget)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[`addWidget`](https://pytest-qt.readthedocs.io/en/latest/reference.html#pytestqt.qtbot.QtBot.addWidget)
[`add_widget`/`addWidget`](https://pytest-qt.readthedocs.io/en/latest/reference.html#pytestqt.qtbot.QtBot.addWidget)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you want both or is there one that you prefer? I guess add_widget would be more 'python-ic'?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks okay as both so will leave

Copy link
Contributor

Choose a reason for hiding this comment

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

In our test we are using both of them. So I would like to signal that both are valid.

signals, etc...). See the [`qtbot` docs](https://pytest-qt.readthedocs.io/en/latest/reference.html#pytestqt.qtbot.QtBot) for details.
This prevents leakage between tests.
The
[`waitUntil`](https://pytest-qt.readthedocs.io/en/latest/reference.html#pytestqt.qtbot.QtBot.waitUntil) method is also useful to wait for a desired condition. The example below
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[`waitUntil`](https://pytest-qt.readthedocs.io/en/latest/reference.html#pytestqt.qtbot.QtBot.waitUntil) method is also useful to wait for a desired condition. The example below
[`wait_until`/`waitUntil`](https://pytest-qt.readthedocs.io/en/latest/reference.html#pytestqt.qtbot.QtBot.waitUntil) method is also useful to wait for a desired condition. The example below

@lucyleeow lucyleeow added this to the 0.5.3 milestone Aug 22, 2024
@lucyleeow
Copy link
Collaborator Author

Thanks @Czaki ! Changes made

@jni jni merged commit 6b74313 into napari:main Aug 29, 2024
8 checks passed
@lucyleeow lucyleeow deleted the qtbot branch August 30, 2024 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0