added success message to run modes, removed useless param, used mocks instead of test specific params #571

sebastian-swob · 2024-11-13T11:39:44Z

No description provided.

removed useless param, used mocks instead of test specific params

ahobeost · 2024-11-14T08:17:30Z

tests/trnsysGUI/storageTank/testConfigureStorageDialog.py

-            mainWindow.editor.trnsysObj, stw.StorageTank
-        )
-        configureStorageDialog = self.storageTank.mouseDoubleClickEvent(
-            _core.QEvent.MouseButtonDblClick, isTest=True


Nice choice. Dealing with the dialog directly keeps this GUI part out of these tests. When we start separating the GUI code properly, these tests should stay valid and the "doubleclickeEvent" would be a minimal GUI test (not for now).

ahobeost · 2024-11-14T08:21:47Z

trnsysGUI/menus/hydraulicModesMenu/hydraulicModes.py

@@ -57,6 +57,11 @@ def runModes(project: prj.CreateOrLoadProject, mainWindow: mw.MainWindow):  # ty
                informativeText=constants.ERROR_RUNNING_MODES_TRNSYS_ADDITIONAL,
                buttons=[_qtw.QMessageBox.Ok],
            )
+        else:
+            MessageBox.create(
+                messageText=f"{constants.SUCCESS_RUNNING_MODES}{','.join(failures)}",


ahobeost · 2024-11-14T08:30:08Z

trnsysGUI/storageTank/widget.py

-    def mouseDoubleClickEvent(
-        self, event: _qtw.QGraphicsSceneMouseEvent, isTest=False
-    ) -> (_tp.Optional)[ConfigureStorageDialog]:
+    def mouseDoubleClickEvent(self, event: _qtw.QGraphicsSceneMouseEvent):


Much better this way, thanks!

Please add return type, too. (-> None, I guess?)

zuckerruebe · 2024-11-14T08:51:31Z

tests/trnsysGUI/storageTank/testConfigureStorageDialog.py

@@ -1,11 +1,13 @@
 import pathlib as _pl

-import PyQt5.QtCore as _core


All imports in pytrnsys/trnsysGUI should be of the form import foo.bar.baz as _alias. In particular, in new code, we don't import individual classes.

zuckerruebe · 2024-11-14T08:53:52Z

tests/trnsysGUI/storageTank/testConfigureStorageDialog.py

+    def triggerStorageDialog(
+        self, qtbot, monkeypatch
+    ) -> ConfigureStorageDialog:
+        monkeypatch.setattr(QMessageBox, "exec", lambda self: QMessageBox.Ok)


Can we maybe patch QMessageBox.information or (warning, or whichever is appropriate, directly?). Might require small changes in productive code.

Thanks @zuckerruebe.

Not sure if that would work.
The main issue is that pytest grinds to a halt until the dialog is closed.
Thus, the problem to solve is:
ensuring that exec() is passed cleanly during testing.

Does the current patch seem better with this information, or would you still prefer patching something else to reach the same goal?

Additionally, I think a bigger overhaul of this is in order.
Two architectural choices would be helpful here.
1: The current state should be saved in memory, so that the entire state can be reproduced at the end of the export.
2: MVP or MVVM, so we do not have to deal with the GUI part at all in these tests.

This would not be part of this PR.

I actually don't know why I get e-mails regarding this pull request. But because I do, I felt I'd chirp in. But I don't have a strong opinion nor a lot of context.

If it's possible to use, e.g., QMessageBox.information, then, if you monkeypatch that one .exec will never get called because it's called within information.

zuckerruebe · 2024-11-14T08:55:11Z

tests/trnsysGUI/storageTank/testConfigureStorageDialog.py

        hxInput = "90"
        hxOutput = "50"
        hxName = "Test hx"
        errors = []

-        configureStorageDialog = self.triggerStorageDialog(qtbot)
+        configureStorageDialog = triggerStorageDialog


triggerStorageDialog is a very confusing name. Just name the parameter configureStorageDialog directly.

@sebastian-swob: I took care of this.

zuckerruebe · 2024-11-14T08:55:25Z

tests/trnsysGUI/storageTank/testConfigureStorageDialog.py

@@ -60,11 +64,11 @@ def testAddHxSuccess(self, qtbot):
        if errors:
            raise ExceptionGroup(f"Found {len(errors)} issues.", errors)

-    def testAddHxMissingNameFailure(self, qtbot):
+    def testAddHxMissingNameFailure(self, triggerStorageDialog):


zuckerruebe · 2024-11-14T08:55:33Z

tests/trnsysGUI/storageTank/testConfigureStorageDialog.py

@@ -75,11 +79,11 @@ def testAddHxMissingNameFailure(self, qtbot):
            == configureStorageDialog.MISSING_NAME_ERROR_MESSAGE
        )

-    def testAddHxInvalidRangeFailure(self, qtbot):
+    def testAddHxInvalidRangeFailure(self, triggerStorageDialog):


zuckerruebe · 2024-11-14T08:55:49Z

tests/trnsysGUI/storageTank/testConfigureStorageDialog.py

@@ -101,23 +105,23 @@ def testRemoveHxSuccess(self, qtbot):

        assert len(self.storageTank.heatExchangers) == 0

-    def testAddPortPairSuccess(self, qtbot):
+    def testAddPortPairSuccess(self, triggerStorageDialog):


zuckerruebe · 2024-11-14T08:55:58Z

tests/trnsysGUI/storageTank/testConfigureStorageDialog.py

        configureStorageDialog.manPortLeI.setText(portPairInput)
        configureStorageDialog.manPortLeO.setText(portPairOutput)
        configureStorageDialog.manlButton.setChecked(True)
        configureStorageDialog.addPortPair()

        assert len(self.storageTank.directPortPairs) == 2

-    def testAddPortPairHeightFailure(self, qtbot):
+    def testAddPortPairHeightFailure(self, triggerStorageDialog):


zuckerruebe · 2024-11-14T08:56:09Z

tests/trnsysGUI/storageTank/testConfigureStorageDialog.py

@@ -128,11 +132,11 @@ def testAddPortPairHeightFailure(self, qtbot):
            == configureStorageDialog.PORT_HEIGHT_ERROR_MESSAGE
        )

-    def testAddPortPairNoSideSelectedFailure(self, qtbot):
+    def testAddPortPairNoSideSelectedFailure(self, triggerStorageDialog):


zuckerruebe · 2024-11-14T08:56:17Z

tests/trnsysGUI/storageTank/testConfigureStorageDialog.py

@@ -143,8 +147,8 @@ def testAddPortPairNoSideSelectedFailure(self, qtbot):
            == configureStorageDialog.NO_SIDE_SELECTED_ERROR_MESSAGE
        )

-    def testRemovePortPairSuccess(self, qtbot):
-        configureStorageDialog = self.triggerStorageDialog(qtbot)
+    def testRemovePortPairSuccess(self, triggerStorageDialog):


zuckerruebe · 2024-11-14T08:57:55Z

trnsysGUI/storageTank/widget.py

-    def mouseDoubleClickEvent(
-        self, event: _qtw.QGraphicsSceneMouseEvent, isTest=False
-    ) -> (_tp.Optional)[ConfigureStorageDialog]:
+    def mouseDoubleClickEvent(self, event: _qtw.QGraphicsSceneMouseEvent):


Please add return type, too. (-> None, I guess?)

…d" as attribute.

sonarcloud · 2024-11-14T15:18:51Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ahobeost · 2024-11-14T15:21:47Z

@sebastian-swob and @zuckerruebe:
Please have another look.

I have resolved the issue with the tempering valves as well.

Explanation:
if isTempering == True
This is turned off, so the regimes can do their thing.
At the end, it is turned back on (for all such valves), so the User does not have to turn it back on Manually

added success message to run modes,

46743ae

removed useless param, used mocks instead of test specific params

sebastian-swob requested review from zuckerruebe and ahobeost and removed request for zuckerruebe November 13, 2024 11:40

This was linked to issues Nov 13, 2024

Some mode checker fixes #573

Closed

Small fix for storage dialog tests #572

Closed

ahobeost reviewed Nov 14, 2024

View reviewed changes

Minor cleanup

9375bb4

ahobeost reviewed Nov 14, 2024

View reviewed changes

zuckerruebe reviewed Nov 14, 2024

View reviewed changes

ahobeost added 8 commits November 14, 2024 10:08

refactored import

48dbb95

Dealt with tempering valves in project.

e4a0cc0

Added initial test for project with tempering valve turned on.

582d699

Fixed issue with resetting tempering valves and finished tests.

869ce8c

Removed manual user request from message.

5f7268c

Added assert when no tempering valves should show up and minor clenanup

a006d92

Black and pylint changes

a1a2a20

Ignoring MyPy "feature" where Sequence sometimes does not have "appen…

6af534c

…d" as attribute.

ahobeost linked an issue Nov 14, 2024 that may be closed by this pull request

Automated export of regimes/modes cannot act on tempering valves #526

Closed

ahobeost added 2 commits November 14, 2024 16:17

Cleanup based on review

20ffd4f

Black changes

d4b3870

ahobeost merged commit 1048041 into master Nov 15, 2024
6 checks passed

ahobeost deleted the tiny-improvements branch November 15, 2024 13:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added success message to run modes, removed useless param, used mocks instead of test specific params #571

added success message to run modes, removed useless param, used mocks instead of test specific params #571

sebastian-swob commented Nov 13, 2024

ahobeost Nov 14, 2024

ahobeost Nov 14, 2024

ahobeost Nov 14, 2024

zuckerruebe Nov 14, 2024

zuckerruebe Nov 14, 2024

zuckerruebe Nov 14, 2024

ahobeost Nov 14, 2024

ahobeost Nov 14, 2024

zuckerruebe Nov 14, 2024

zuckerruebe Nov 14, 2024

ahobeost Nov 14, 2024

zuckerruebe Nov 14, 2024

zuckerruebe Nov 14, 2024

zuckerruebe Nov 14, 2024

zuckerruebe Nov 14, 2024

zuckerruebe Nov 14, 2024

zuckerruebe Nov 14, 2024

zuckerruebe Nov 14, 2024

sonarcloud bot commented Nov 14, 2024

ahobeost commented Nov 14, 2024

		@@ -1,11 +1,13 @@
		import pathlib as _pl

		import PyQt5.QtCore as _core

added success message to run modes, removed useless param, used mocks instead of test specific params #571

added success message to run modes, removed useless param, used mocks instead of test specific params #571

Conversation

sebastian-swob commented Nov 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Nov 14, 2024

Quality Gate passed

ahobeost commented Nov 14, 2024