Skip to content
This repository was archived by the owner on Nov 6, 2025. It is now read-only.

Just pass the tests - #113

Merged
seb5g merged 13 commits into
PyMoDAQ:5.0.x_devfrom
malik-irain:fix/pass-the-tests
Oct 30, 2025
Merged

seb5g merged 13 commits into
PyMoDAQ:5.0.x_devfrom
malik-irain:fix/pass-the-tests

Conversation

@malik-irain

Copy link
Copy Markdown
Contributor

Finally solved!

@codecov

codecov Bot commented Oct 27, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.28%. Comparing base (3a1a81b) to head (1499d2a).
⚠️ Report is 14 commits behind head on 5.0.x_dev.

Additional details and impacted files
@@              Coverage Diff              @@
##           5.0.x_dev     #113      +/-   ##
=============================================
+ Coverage      64.24%   64.28%   +0.04%     
=============================================
  Files             73       73              
  Lines           9137     9156      +19     
=============================================
+ Hits            5870     5886      +16     
- Misses          3267     3270       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@seb5g seb5g left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could you tell what you did a bit more explicitly?

return descriptors


class ROIMixin(QtCore.QObject):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if it's not inheriting from QObject one cannot create and use signal. I guess because it's a mixin, it's no big deal if used within another class inheriting from QObject? Or maybe that was the issue?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As it's a Mixin class it's not supposed to be used by itself and it's actually used by another class inheriting QObject. So in the end, QObject was initialized twice. When done in normal execution it's fine (or at least it doesn't crash) but in pytest it seems to be the source of segfaults.

I tried several ways to solve the problem but this was the only functional one.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok understood! THere are still tests not working ;-)

@malik-irain

Copy link
Copy Markdown
Contributor Author

One problem I have and can't seem to solve is with the test search_lineedit_test.py::TestSearchLineEdit::test_debounce_resets_on_new_input
https://github.com/PyMoDAQ/pymodaq_gui/blob/5.0.x_dev/tests/utils/widgets/search_lineedit_test.py#L47-L56

@Ashwolaa How is it different than the one directly before, test_rapid_changes_emit_once in what's tested. Both seem to test that only the last set text is taken into account as long as it done before debounce timeout, no?

@Ashwolaa

Copy link
Copy Markdown
Contributor

One problem I have and can't seem to solve is with the test search_lineedit_test.py::TestSearchLineEdit::test_debounce_resets_on_new_input https://github.com/PyMoDAQ/pymodaq_gui/blob/5.0.x_dev/tests/utils/widgets/search_lineedit_test.py#L47-L56

Hum strange. With PyQt6, the tests are passing on my config (fresh install though).

@Ashwolaa How is it different than the one directly before, test_rapid_changes_emit_once in what's tested. Both seem to test that only the last set text is taken into account as long as it done before debounce timeout, no?

Yeah, that is correct. One is just checking by stacking all the signals while the other is only waiting for the first one to come but they are roughly equivalent. We can remove/merge it if you prefer.

@malik-irain

Copy link
Copy Markdown
Contributor Author

Yes, the issue only appear with PySide6, but I'm not sure the test is wrong, because even when replacing it with pass, doesn't avoid triggered the Qt Error (RuntimeError: Internal C++ object (PySide6.QtWidgets.QWidgetAction) already deleted.) . So I'm thinking maybe some Qt Object isn't attached correctly in the implementation

@malik-irain
malik-irain requested a review from seb5g October 30, 2025 14:08
@malik-irain

Copy link
Copy Markdown
Contributor Author

Should be good, everything pass now!

@Ashwolaa

Copy link
Copy Markdown
Contributor

You fixed pyside6!
Nice job !

@seb5g
seb5g merged commit 9f7bad7 into PyMoDAQ:5.0.x_dev Oct 30, 2025
28 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants