Adds test coverage for drag&drop functionality

[miker2]

No description provided.

[gemini-code-assist]

Hello @miker2, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello team,

Gemini here, providing a summary of this pull request authored by @miker2. The primary goal of this PR, as indicated by the title, is to add comprehensive test coverage for the drag-and-drop functionality within the application's plotting widgets, specifically focusing on the SubPlotWidget and CustomPlotItem components. The changes involve both minor adjustments to the existing drag handling code in custom_plot_item.py to improve testability and, more significantly, the addition of a new suite of tests in tests/test_sub_plot_widget.py to cover various drag-and-drop scenarios. The tests leverage pytest-qt and mocking techniques to simulate user interactions and verify the expected behavior.

Highlights

• Test Coverage: Adds extensive new test cases for the drag-and-drop features in SubPlotWidget.
• Drag & Drop Scenarios Tested: Includes tests for dragging new data items onto a subplot, reordering existing traces within a subplot, moving traces between different subplots, dragging traces to the plot area to append them, and verifying that drags with unrecognized mime types are ignored.
• Testing Framework: Utilizes pytest-qt's qtbot for simulating GUI interactions and mocker for patching methods like QDrag.exec_ to control the drag-and-drop flow in tests.
• Code Adjustments: Makes minor adjustments to CustomPlotItem's drag handling, such as initializing the drag start position as a QPoint and using self.grab() for the drag pixmap, primarily to facilitate testing.
• Test Helpers: Introduces mock classes (MockDraggableVarItem, MockDraggableUnrecognizedItem) and helper functions (_add_traces_to_subplot, _assert_trace_order_and_colors, create_data_item_mime_data, create_unrecognized_mime_data) to streamline test setup and verification.

Changelog

Click here to see the changelog

• custom_plot_item.py
  • Added import for QPoint from PyQt5.QtCore.
  • Adjusted the import of QApplication to directly import from PyQt5.QtWidgets and removed the try...except block.
  • Changed the initialization of _drag_start_position from None to QPoint() and renamed the attribute to drag_start_position.
  • Updated mousePressEvent and mouseMoveEvent to use the new self.drag_start_position attribute.
  • Added checks in mouseMoveEvent to ensure drag_start_position is valid before initiating a drag.
  • Replaced QPixmap(self.size()) and self.render(pixmap) with self.grab() for creating the drag pixmap.
  • Cleaned up try...except blocks around mime data setting and drag.exec_() calls, adding logging outside the blocks.
  • Ensured drag_start_position is reset to QPoint() after drag.exec_().
• tests/test_sub_plot_widget.py
  • Added necessary imports for Qt drag and drop testing classes (QEvent, QDragEnterEvent, QDragMoveEvent, QDropEvent, QDrag, QMouseEvent, QApplication, QWidget, pickle).
  • Added a var_name attribute to the MockDataSource class.
  • Introduced MockDraggableVarItem and MockDraggableUnrecognizedItem classes to simulate different drag sources.
  • Added helper functions _add_traces_to_subplot and _assert_trace_order_and_colors for test setup and assertions.
  • Added helper functions create_data_item_mime_data and create_unrecognized_mime_data for creating mock mime data.
  • Removed obsolete event creation helper functions.
  • Implemented new test functions using qtbot and mocker to cover various drag-and-drop scenarios: test_drag_data_item_to_subplot, test_reorder_custom_plot_item_same_subplot, test_move_custom_plot_item_between_subplots, test_drag_custom_plot_item_to_plot_area_appends, and test_drag_unrecognized_mime_type_is_ignored.
  • Updated existing test function definitions to include the qtbot fixture.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

Drag a trace with ease,
Across the plot, if you please.
Tests now confirm,
No bugs will squirm,
GUI moves bring sweet release.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces comprehensive test coverage for drag-and-drop functionality, which is a valuable addition. The new tests in tests/test_sub_plot_widget.py cover various scenarios, and the changes in custom_plot_item.py refine the drag-and-drop implementation. The use of qtbot and mocker are well-executed.

Summary of Findings

• Error Handling in CustomPlotItem.mouseMoveEvent: Removed specific try-except blocks around MIME data creation. If self.trace.name() returns None, self.trace.name().encode() will raise an AttributeError. This could lead to unhandled exceptions during drag operations. Suggests re-adding error handling or checks for None.
• Attribute Naming Convention (Minor/Not Commented): The attribute _drag_start_position was changed to drag_start_position (public). If this change is primarily to facilitate testing, it's a common practice. However, if not strictly necessary for external access beyond testing, maintaining it as a protected member (_drag_start_position) would align better with encapsulation principles. This was not commented on due to review settings for severity.

Merge Readiness

The pull request significantly improves test coverage and is generally in good shape. However, I recommend addressing the high and medium-severity issues regarding error handling in custom_plot_item.py to ensure robustness during drag-and-drop operations. Once those points are considered, the PR should be in a strong position for merging. As a reviewer, I am not authorized to approve pull requests; please ensure further review and approval by authorized maintainers.

[gemini-code-assist]

The previous version of this code had specific try-except blocks around mime_data.setText() and mime_data.setData() calls, logging errors if they occurred. The current refactoring has removed these.

If self.trace.name() were to return None (which PlotDataItem.name() can, as it returns self.opts.get('name', None)), then self.trace.name().encode() would raise an AttributeError ('NoneType' object has no attribute 'encode'). This could cause an unhandled exception during the drag operation, potentially crashing or destabilizing the drag. Can we consider adding a check for self.trace.name() being None or reintroducing a broader try-except block around the MIME data population to catch potential exceptions and log them, ensuring the drag operation fails gracefully or can be aborted cleanly?

if self.trace.name() is not None:
    mime_data.setData("application/x-customplotitem", self.trace.name().encode())
else:
    mime_data.setData("application/x-customplotitem", QByteArray())

[gemini-code-assist]

The previous version of this code had specific try-except blocks around mime_data.setText() and mime_data.setData() calls, logging errors if they occurred. The current refactoring has removed these.

If self.trace.name() were to return None (which PlotDataItem.name() can, as it returns self.opts.get('name', None)), then mime_data.setText(self.trace.name()) might pass None to Qt, which could lead to unexpected behavior or errors depending on Qt's handling of None for string conversion. Can we consider adding a check for self.trace.name() being None or reintroducing a broader try-except block around the MIME data population to catch potential exceptions and log them, ensuring the drag operation fails gracefully or can be aborted cleanly?

if self.trace.name() is not None:
    mime_data.setText(self.trace.name())
else:
    mime_data.setText("") # Or some default string