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

MNT: check CIs #136

Merged
merged 26 commits into from
Dec 15, 2021
Merged

MNT: check CIs #136

merged 26 commits into from
Dec 15, 2021

Conversation

GuillaumeFavelier
Copy link
Contributor

@GuillaumeFavelier GuillaumeFavelier commented Dec 14, 2021

I am just doing a routine check of the CIs before the release. I noticed failures on LinuxConda and Linux Python37 jobs. Hopefully, it is just something minor like adjusting timeouts etc.

2021-12-14T13:29:57.0447323Z =================================== FAILURES ===================================
2021-12-14T13:29:57.0448358Z ____________________ test_background_plotting_add_callback _____________________
2021-12-14T13:29:57.0449063Z 
2021-12-14T13:29:57.0449873Z qtbot = <pytestqt.qtbot.QtBot object at 0x7fdc342a91f0>
2021-12-14T13:29:57.0451088Z monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x7fdc342a9880>
2021-12-14T13:29:57.0451952Z plotting = None
2021-12-14T13:29:57.0452429Z 
2021-12-14T13:29:57.0455482Z     def test_background_plotting_add_callback(qtbot, monkeypatch, plotting):
2021-12-14T13:29:57.0456299Z         class CallBack(object):
2021-12-14T13:29:57.0456782Z             def __init__(self, sphere):
2021-12-14T13:29:57.0457495Z                 self.sphere = sphere
2021-12-14T13:29:57.0457850Z     
2021-12-14T13:29:57.0458194Z             def __call__(self):
2021-12-14T13:29:57.0458591Z                 self.sphere.points *= 0.5
2021-12-14T13:29:57.0458929Z     
2021-12-14T13:29:57.0459246Z         update_count = [0]
2021-12-14T13:29:57.0459661Z         orig_update_app_icon = BackgroundPlotter.update_app_icon
2021-12-14T13:29:57.0460043Z     
2021-12-14T13:29:57.0460381Z         def update_app_icon(slf):
2021-12-14T13:29:57.0460776Z             update_count[0] = update_count[0] + 1
2021-12-14T13:29:57.0461195Z             return orig_update_app_icon(slf)
2021-12-14T13:29:57.0461537Z     
2021-12-14T13:29:57.0476893Z         monkeypatch.setattr(BackgroundPlotter, 'update_app_icon', update_app_icon)
2021-12-14T13:29:57.0477970Z         plotter = BackgroundPlotter(
2021-12-14T13:29:57.0478532Z             show=False,
2021-12-14T13:29:57.0479366Z             off_screen=False,
2021-12-14T13:29:57.0480181Z             title='Testing Window',
2021-12-14T13:29:57.0480924Z             update_app_icon=True,  # also does add_callback
2021-12-14T13:29:57.0482362Z         )
2021-12-14T13:29:57.0483846Z         assert plotter._last_update_time == -np.inf
2021-12-14T13:29:57.0484714Z         sphere = pyvista.Sphere()
2021-12-14T13:29:57.0485275Z         mycallback = CallBack(sphere)
2021-12-14T13:29:57.0485819Z         plotter.add_mesh(sphere)
2021-12-14T13:29:57.0486401Z         plotter.add_callback(mycallback, interval=200, count=3)
2021-12-14T13:29:57.0486986Z     
2021-12-14T13:29:57.0487506Z         # check that timers are set properly in add_callback()
2021-12-14T13:29:57.0488132Z         assert_hasattr(plotter, "app_window", MainWindow)
2021-12-14T13:29:57.0488861Z         assert_hasattr(plotter, "_callback_timer", QTimer)
2021-12-14T13:29:57.0489489Z         assert_hasattr(plotter, "counters", list)
2021-12-14T13:29:57.0490006Z     
2021-12-14T13:29:57.0490522Z         window = plotter.app_window  # MainWindow
2021-12-14T13:29:57.0491115Z         callback_timer = plotter._callback_timer  # QTimer
2021-12-14T13:29:57.0492261Z         counter = plotter.counters[-1]  # Counter
2021-12-14T13:29:57.0492851Z     
2021-12-14T13:29:57.0493360Z         # ensure that the window is showed
2021-12-14T13:29:57.0493911Z         assert not window.isVisible()
2021-12-14T13:29:57.0494476Z         with qtbot.wait_exposed(window):
2021-12-14T13:29:57.0495229Z             window.show()
2021-12-14T13:29:57.0495736Z         assert window.isVisible()
2021-12-14T13:29:57.0496311Z         assert update_count[0] in [0, 1]  # macOS sometimes updates (1)
2021-12-14T13:29:57.0497193Z         # don't check _last_update_time for non-inf-ness, won't be updated on Win
2021-12-14T13:29:57.0498159Z         plotter.update_app_icon()  # the timer doesn't call it right away, so do it
2021-12-14T13:29:57.0499032Z         assert update_count[0] in [1, 2]
2021-12-14T13:29:57.0499842Z         plotter.update_app_icon()  # should be a no-op
2021-12-14T13:29:57.0500475Z         assert update_count[0] in [2, 3]
2021-12-14T13:29:57.0501105Z         with pytest.raises(ValueError, match="ndarray with shape"):
2021-12-14T13:29:57.0501887Z             plotter.set_icon(0.)
2021-12-14T13:29:57.0502684Z         # Maybe someday manually setting "set_icon" should disable update_app_icon?
2021-12-14T13:29:57.0503351Z         # Strings also supported directly by QIcon
2021-12-14T13:29:57.0503975Z         plotter.set_icon(os.path.join(
2021-12-14T13:29:57.0504575Z             os.path.dirname(pyvistaqt.__file__), "data",
2021-12-14T13:29:57.0505324Z             "pyvista_logo_square.png"))
2021-12-14T13:29:57.0505815Z     
2021-12-14T13:29:57.0506343Z         # ensure that self.callback_timer send a signal
2021-12-14T13:29:57.0506997Z         callback_blocker = qtbot.wait_signals([callback_timer.timeout], timeout=300)
2021-12-14T13:29:57.0507813Z         callback_blocker.wait()
2021-12-14T13:29:57.0508540Z         # ensure that self.counters send a signal
2021-12-14T13:29:57.0509179Z         counter_blocker = qtbot.wait_signals([counter.signal_finished], timeout=700)
2021-12-14T13:29:57.0509803Z >       counter_blocker.wait()
2021-12-14T13:29:57.0510465Z E       pytestqt.exceptions.TimeoutError: Emitted signals: None. Missing: [signal_finished()]

@GuillaumeFavelier GuillaumeFavelier self-assigned this Dec 14, 2021
@codecov
Copy link

codecov bot commented Dec 14, 2021

Codecov Report

Merging #136 (df8696a) into main (d81885c) will decrease coverage by 0.91%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #136      +/-   ##
==========================================
- Coverage   98.17%   97.26%   -0.92%     
==========================================
  Files           8        8              
  Lines         658      658              
==========================================
- Hits          646      640       -6     
- Misses         12       18       +6     

@GuillaumeFavelier
Copy link
Contributor Author

Here is the situation:

  1. CI / macos-latest / conda / PySide2 is stuck in limbo and I have no idea why.
  2. From time to time, Linux Python36 cannot pass test_background_plotting_add_callback(). I tried this one locally and I cannot reproduce.
  3. Occasionally, CI / macos-latest / pip / PySide2 / crashes. Still it happens too much to my liking. But I noticed that those crashes appears during the calls of qtbot.wait_signals() (not sure how it could be useful for now).

@GuillaumeFavelier GuillaumeFavelier mentioned this pull request Dec 14, 2021
@GuillaumeFavelier
Copy link
Contributor Author

To be precise, 3) does not happen only in qtbot.wait_signals() but on the explicit call to wait(). My first idea is then to refactor those to use the context manager instead.

@GuillaumeFavelier
Copy link
Contributor Author

After trying multiple things:

  • show=False
  • off_screen=False
  • auto_update=False
  • force _render() or not
  • without empty_scene()

It's not entirely clear why the crash does happen. Although I learned where it comes from or rather where it is detected:
the __exit__() function of the _WaitWidgetContextManager class in pytest-qt (I inspected it and I do not see anything wrong here).

I still think it is somehow related to QTimers and/or rendering paintEvent()/render().


To move forward with the PR:

  • I changed the Qt binding to PyQt5 on MacOS. After all, PySide2 is not updated on PyPi since last year (11/20/2020). And this solves 1. and 3.
  • 2. does still happens occasionally but restarting should fix this

Apart from that, I modified test_background_plotting_add_callback() to test only one timer at a time, added stages on Azure to support [skip azp] and updated the GitHub Actions workflow.

This is ready to go from my end @pyvista/developers

Comment on lines +89 to +92
if: matrix.os == 'macos-latest'
- run: pip install PySide2
name: 'Install Qt binding'
if: matrix.os != 'macos-latest'
Copy link
Contributor

Choose a reason for hiding this comment

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

Any way to keep testing pyside2 on Linux as well? Maybe the matrix should be macos-pyqt5, linux-pyqt5, linux-pyside2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PySide2 is still tested on linux and windows for PyPI jobs.
PyQt5 is set for conda jobs though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh okay, I misread the config -- great!

Copy link
Contributor

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM. Feel free to merge if you don't think PySide2 on linux is a good idea, or want to tackle it in another PR @GuillaumeFavelier !

@larsoner
Copy link
Contributor

... on second thought, I'll go ahead and merge so you can push out a release @GuillaumeFavelier , let's tackle the linux+pyside2 in another PR

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.

None yet

2 participants