From e6d9ad31ab0e432a7357dfa702332cae25aead88 Mon Sep 17 00:00:00 2001 From: Lipu Fei Date: Tue, 20 Nov 2018 09:20:45 +0100 Subject: [PATCH 01/11] Use generated Makefiles to run tests --- Jenkinsfile | 43 ++----------------------------------------- 1 file changed, 2 insertions(+), 41 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index f9a3a9864a..a345ebbd05 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -38,20 +38,9 @@ parallel_nodes(['linux && cura', 'windows && cura']) { if (isUnix()) { - // For Linux to show everything - def branch = env.BRANCH_NAME - if(!fileExists("${env.CURA_ENVIRONMENT_PATH}/${branch}")) - { - branch = "master" - } - def uranium_dir = get_workspace_dir("Ultimaker/Uranium/${branch}") - + // For Linux try { - sh """ - cd .. - export PYTHONPATH=.:"${uranium_dir}" - ${env.CURA_ENVIRONMENT_PATH}/${branch}/bin/pytest -x --verbose --full-trace --capture=no ./tests - """ + sh 'make CTEST_OUTPUT_ON_FAILURE=TRUE test' } catch(e) { currentBuild.result = "UNSTABLE" @@ -70,34 +59,6 @@ parallel_nodes(['linux && cura', 'windows && cura']) } } } - - stage('Code Style') - { - if (isUnix()) - { - // For Linux to show everything. - // CMake also runs this test, but if it fails then the test just shows "failed" without details of what exactly failed. - def branch = env.BRANCH_NAME - if(!fileExists("${env.CURA_ENVIRONMENT_PATH}/${branch}")) - { - branch = "master" - } - def uranium_dir = get_workspace_dir("Ultimaker/Uranium/${branch}") - - try - { - sh """ - cd .. - export PYTHONPATH=.:"${uranium_dir}" - ${env.CURA_ENVIRONMENT_PATH}/${branch}/bin/python3 run_mypy.py - """ - } - catch(e) - { - currentBuild.result = "UNSTABLE" - } - } - } } } From 76f2aeb43c8ab19a638ade5016229a5f542664cb Mon Sep 17 00:00:00 2001 From: Diego Prado Gesto Date: Tue, 20 Nov 2018 11:27:45 +0100 Subject: [PATCH 02/11] Fix the title's top margin size in the add machine dialog. --- resources/qml/AddMachineDialog.qml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/qml/AddMachineDialog.qml b/resources/qml/AddMachineDialog.qml index 0df8b891d9..aa160acd4d 100644 --- a/resources/qml/AddMachineDialog.qml +++ b/resources/qml/AddMachineDialog.qml @@ -73,7 +73,7 @@ UM.Dialog { top: parent.top left: parent.left - topMargin: UM.Theme.getSize("default_margin") + topMargin: UM.Theme.getSize("default_margin").height } text: catalog.i18nc("@title:tab", "Add a printer to Cura") From 481ca8cd2f2e61ba7591695c7b318346cad4364d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Tue, 20 Nov 2018 16:33:52 +0100 Subject: [PATCH 03/11] Fixed some bugs and added the color_code field to the named tuple --- plugins/UM3NetworkPrinting/src/Models.py | 1 + .../UM3NetworkPrinting/src/SendMaterialJob.py | 20 +++++++++++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index e8efa577f6..a9210ac5b4 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -23,6 +23,7 @@ LocalMaterial = namedtuple('LocalMaterial', [ 'brand', 'material', 'color_name', + 'color_code', 'description', 'adhesion_info', 'approximate_diameter', diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index cbe79aef6a..0599101379 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -2,6 +2,7 @@ # Cura is released under the terms of the LGPLv3 or higher. import json import os +import re import urllib.parse from typing import Dict, TYPE_CHECKING, Set @@ -19,7 +20,6 @@ from .Models import ClusterMaterial, LocalMaterial if TYPE_CHECKING: from .ClusterUM3OutputDevice import ClusterUM3OutputDevice - ## Asynchronous job to send material profiles to the printer. # # This way it won't freeze up the interface while sending those materials. @@ -50,7 +50,7 @@ class SendMaterialJob(Job): self._sendMissingMaterials(remote_materials_by_guid) except json.JSONDecodeError: Logger.logException("w", "Error parsing materials from printer") - except KeyError: + except TypeError: Logger.logException("w", "Error parsing materials from printer") ## Determine which materials should be updated and send them to the printer. @@ -75,7 +75,8 @@ class SendMaterialJob(Job): ## From the local and remote materials, determine which ones should be synchronized. # - # Makes a Set containing only the materials that are not on the printer yet or the ones that are newer in Cura. + # Makes a Set of id's containing only the id's of the materials that are not on the printer yet or the ones that + # are newer in Cura. # # \param local_materials The local materials by GUID. # \param remote_materials The remote materials by GUID. @@ -157,7 +158,7 @@ class SendMaterialJob(Job): @classmethod def _parseReply(cls, reply: QNetworkReply) -> Dict[str, ClusterMaterial]: remote_materials = json.loads(reply.readAll().data().decode("utf-8")) - return {material["id"]: ClusterMaterial(**material) for material in remote_materials} + return {material["guid"]: ClusterMaterial(**material) for material in remote_materials} ## Retrieves a list of local materials # @@ -170,12 +171,19 @@ class SendMaterialJob(Job): material_containers = container_registry.findContainersMetadata(type = "material") # Find the latest version of all material containers in the registry. - local_materials = {} # type: Dict[str, LocalMaterial] for material in material_containers: try: material = LocalMaterial(**material) + + # material version must be an int + if not re.match("\d+", material.version): + Logger.logException("w", "Local material {} has invalid version '{}'." + .format(material["id"], material.version)) + continue + if material.GUID not in result or material.version > result.get(material.GUID).version: - local_materials[material.GUID] = material + result[material.GUID] = material except ValueError: Logger.logException("w", "Local material {} has invalid values.".format(material["id"])) + return result From ca6074429290b6ae5fa1f3ccfbfa74107ed1284a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Tue, 20 Nov 2018 16:34:11 +0100 Subject: [PATCH 04/11] Made the tests work with the named tuples Tests only use the _onGetRemoteMaterial --- .../tests/TestSendMaterialJob.py | 313 +++++++----------- 1 file changed, 123 insertions(+), 190 deletions(-) diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index 73bca2b0ad..f4604580fe 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -1,53 +1,23 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. +import io import json - -from typing import Any, List -from unittest import TestCase +from unittest import TestCase, mock from unittest.mock import patch, call from PyQt5.QtCore import QByteArray -from UM.Settings.ContainerRegistry import ContainerInterface, ContainerRegistryInterface, DefinitionContainerInterface -from plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice import ClusterUM3OutputDevice +from UM.MimeTypeDatabase import MimeType +from cura.CuraApplication import CuraApplication from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob -class ContainerRegistryMock(ContainerRegistryInterface): - - def __init__(self): - self.containersMetaData = None - - def findContainers(self, *, ignore_case: bool = False, **kwargs: Any) -> List[ContainerInterface]: - raise NotImplementedError() - - def findDefinitionContainers(self, **kwargs: Any) -> List[DefinitionContainerInterface]: - raise NotImplementedError() - - @classmethod - def getApplication(cls) -> "Application": - raise NotImplementedError() - - def getEmptyInstanceContainer(self) -> "InstanceContainer": - raise NotImplementedError() - - def isReadOnly(self, container_id: str) -> bool: - raise NotImplementedError() - - def setContainersMetadata(self, value): - self.containersMetaData = value - - def findContainersMetadata(self, type): - return self.containersMetaData - - -class MockOutputDevice(ClusterUM3OutputDevice): - def _createFormPart(self, content_header, data, content_type=None): - return "xxx" - - +@patch("builtins.open", lambda _, __: io.StringIO("")) +@patch("UM.MimeTypeDatabase.MimeTypeDatabase.getMimeTypeForFile", + lambda _: MimeType(name = "application/x-ultimaker-material-profile", comment = "Ultimaker Material Profile", + suffixes = ["xml.fdm_material"])) +@patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: ["/materials/generic_pla_white.xml.fdm_material"]) class TestSendMaterialJob(TestCase): - _LOCAL_MATERIAL_WHITE = {"type": "material", "status": "unknown", "id": "generic_pla_white", "base_file": "generic_pla_white", "setting_version": 5, "name": "White PLA", "brand": "Generic", "material": "PLA", "color_name": "White", @@ -88,11 +58,11 @@ class TestSendMaterialJob(TestCase): job.run() # We expect the materials endpoint to be called when the job runs. - device_mock.get.assert_called_with("materials/", on_finished=job._onGetRemoteMaterials) + device_mock.get.assert_called_with("materials/", on_finished = job._onGetRemoteMaterials) @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_withFailedRequest(self, reply_mock, device_mock): + def test__onGetRemoteMaterials_withFailedRequest(self, reply_mock, device_mock): reply_mock.attribute.return_value = 404 job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) @@ -103,7 +73,7 @@ class TestSendMaterialJob(TestCase): @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_withBadJsonAnswer(self, reply_mock, device_mock): + def test__onGetRemoteMaterials_withBadJsonAnswer(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(b"Six sick hicks nick six slick bricks with picks and sticks.") job = SendMaterialJob(device_mock) @@ -116,7 +86,7 @@ class TestSendMaterialJob(TestCase): @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_withMissingGuid(self, reply_mock, device_mock): + def test__onGetRemoteMaterials_withMissingGuidInRemoteMaterial(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 remote_material_without_guid = self._REMOTE_MATERIAL_WHITE.copy() del remote_material_without_guid["guid"] @@ -127,151 +97,114 @@ class TestSendMaterialJob(TestCase): # We expect the reply to be called once to try to get the printers from the list (readAll()). # Given that parsing fails we do not expect the device to be called for any follow up. self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) - self.assertEqual(1, device_mock.createFormPart.call_count) + self.assertEqual(0, device_mock.createFormPart.call_count) - # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_sendMissingMaterials_WithInvalidVersionInLocalMaterial(self, reply_mock): - # reply_mock.attribute.return_value = 200 - # reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - # - # containerRegistry = ContainerRegistryMock() - # localMaterialWhiteWithInvalidVersion = self._LOCALMATERIAL_WHITE.copy() - # localMaterialWhiteWithInvalidVersion["version"] = "one" - # containerRegistry.setContainersMetadata([localMaterialWhiteWithInvalidVersion]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # SendMaterialJob(None).sendMissingMaterials(reply_mock) - # - # reply_mock.attribute.assert_called_with(0) - # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - # self._assertLogEntries([("e", "Material generic_pla_white has invalid version number one.")], _logentries) - # - # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_sendMissingMaterials_WithMultipleLocalVersionsLowFirst(self, reply_mock): - # reply_mock.attribute.return_value = 200 - # reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - # - # containerRegistry = ContainerRegistryMock() - # localMaterialWhiteWithHigherVersion = self._LOCALMATERIAL_WHITE.copy() - # localMaterialWhiteWithHigherVersion["version"] = "2" - # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWhiteWithHigherVersion]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # SendMaterialJob(None).sendMissingMaterials(reply_mock) - # - # reply_mock.attribute.assert_called_with(0) - # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - # self._assertLogEntries([], _logentries) - # - # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_sendMissingMaterials_MaterialMissingOnPrinter(self, reply_mock): - # reply_mock.attribute.return_value = 200 - # reply_mock.readAll.return_value = QByteArray( - # json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - # - # containerRegistry = ContainerRegistryMock() - # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # SendMaterialJob(None).sendMissingMaterials(reply_mock) - # - # reply_mock.attribute.assert_called_with(0) - # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - # self._assertLogEntries([], _logentries) - # - # @patch("builtins.open", lambda a, b: io.StringIO("")) - # @patch("UM.MimeTypeDatabase.MimeTypeDatabase.getMimeTypeForFile", - # lambda _: MimeType(name="application/x-ultimaker-material-profile", comment="Ultimaker Material Profile", - # suffixes=["xml.fdm_material"])) - # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: ["/materials/generic_pla_white.xml.fdm_material"]) - # @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - # def test_sendMaterialsToPrinter(self, device_mock): - # device_mock._createFormPart.return_value = "_xXx_" - # with mock.patch.object(Logger, "log", new=new_log): - # job = SendMaterialJob(device_mock) - # job.sendMaterialsToPrinter({"generic_pla_white"}) - # - # self._assertLogEntries([("d", "Syncing material generic_pla_white with cluster.")], _logentries) - # self.assertEqual([call._createFormPart("name="file"; filename="generic_pla_white.xml.fdm_material"", ""), - # call.postFormWithParts(on_finished=job.sendingFinished, parts = ["_xXx_"], target = "materials/")], device_mock.method_calls) - # - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_sendingFinished_success(self, reply_mock) -> None: - # reply_mock.attribute.return_value = 200 - # with mock.patch.object(Logger, "log", new=new_log): - # SendMaterialJob(None).sendingFinished(reply_mock) - # - # reply_mock.attribute.assert_called_once_with(0) - # self.assertEqual(0, len(_logentries)) - # - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_sendingFinished_failed(self, reply_mock) -> None: - # reply_mock.attribute.return_value = 404 - # reply_mock.readAll.return_value = QByteArray(b"Six sick hicks nick six slick bricks with picks and sticks.") - # - # with mock.patch.object(Logger, "log", new=new_log): - # SendMaterialJob(None).sendingFinished(reply_mock) - # - # reply_mock.attribute.assert_called_with(0) - # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.attribute(0), call.readAll()]) - # - # self._assertLogEntries([ - # ("e", "Received error code from printer when syncing material: 404"), - # ("e", "Six sick hicks nick six slick bricks with picks and sticks.") - # ], _logentries) - # - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_parseReply(self, reply_mock): - # reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - # - # response = SendMaterialJob._parseReply(reply_mock) - # - # self.assertTrue(len(response) == 1) - # self.assertEqual(next(iter(response.values())), ClusterMaterial(**self._REMOTEMATERIAL_WHITE)) - # - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_parseReplyWithInvalidMaterial(self, reply_mock): - # remoteMaterialWithInvalidVersion = self._REMOTEMATERIAL_WHITE.copy() - # remoteMaterialWithInvalidVersion["version"] = "one" - # reply_mock.readAll.return_value = QByteArray(json.dumps([remoteMaterialWithInvalidVersion]).encode("ascii")) - # - # with self.assertRaises(ValueError): - # SendMaterialJob._parseReply(reply_mock) - # - # def test__getLocalMaterials(self): - # containerRegistry = ContainerRegistryMock() - # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # local_materials = SendMaterialJob(None)._getLocalMaterials() - # - # self.assertTrue(len(local_materials) == 2) - # - # def test__getLocalMaterialsWithMultipleVersions(self): - # containerRegistry = ContainerRegistryMock() - # localMaterialWithNewerVersion = self._LOCALMATERIAL_WHITE.copy() - # localMaterialWithNewerVersion["version"] = 2 - # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWithNewerVersion]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # local_materials = SendMaterialJob(None)._getLocalMaterials() - # - # self.assertTrue(len(local_materials) == 1) - # self.assertTrue(list(local_materials.values())[0].version == 2) - # - # containerRegistry.setContainersMetadata([localMaterialWithNewerVersion, self._LOCALMATERIAL_WHITE]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # local_materials = SendMaterialJob(None)._getLocalMaterials() - # - # self.assertTrue(len(local_materials) == 1) - # self.assertTrue(list(local_materials.values())[0].version == 2) + @patch("cura.Settings.CuraContainerRegistry") + @patch("cura.CuraApplication") + @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") + @patch("PyQt5.QtNetwork.QNetworkReply") + def test__onGetRemoteMaterials_withInvalidVersionInLocalMaterial(self, reply_mock, device_mock, application_mock, + container_registry_mock): + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii")) + + localMaterialWhiteWithInvalidVersion = self._LOCAL_MATERIAL_WHITE.copy() + localMaterialWhiteWithInvalidVersion["version"] = "one" + container_registry_mock.findContainersMetadata.return_value = [localMaterialWhiteWithInvalidVersion] + + application_mock.getContainerRegistry.return_value = container_registry_mock + + with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) + self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) + self.assertEqual(0, device_mock.createFormPart.call_count) + + @patch("cura.Settings.CuraContainerRegistry") + @patch("cura.CuraApplication") + @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") + @patch("PyQt5.QtNetwork.QNetworkReply") + def test__onGetRemoteMaterials_withNoUpdate(self, reply_mock, device_mock, application_mock, + container_registry_mock): + application_mock.getContainerRegistry.return_value = container_registry_mock + + device_mock.createFormPart.return_value = "_xXx_" + + container_registry_mock.findContainersMetadata.return_value = [self._LOCAL_MATERIAL_WHITE] + + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii")) + + with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) + self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) + self.assertEqual(0, device_mock.createFormPart.call_count) + self.assertEqual(0, device_mock.postFormWithParts.call_count) + + @patch("cura.Settings.CuraContainerRegistry") + @patch("cura.CuraApplication") + @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") + @patch("PyQt5.QtNetwork.QNetworkReply") + def test__onGetRemoteMaterials_withUpdatedMaterial(self, reply_mock, device_mock, application_mock, + container_registry_mock): + application_mock.getContainerRegistry.return_value = container_registry_mock + + device_mock.createFormPart.return_value = "_xXx_" + + localMaterialWhiteWithHigherVersion = self._LOCAL_MATERIAL_WHITE.copy() + localMaterialWhiteWithHigherVersion["version"] = "2" + container_registry_mock.findContainersMetadata.return_value = [localMaterialWhiteWithHigherVersion] + + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii")) + + with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) + self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) + self.assertEqual(1, device_mock.createFormPart.call_count) + self.assertEqual(1, device_mock.postFormWithParts.call_count) + self.assertEquals( + [call.createFormPart("name=\"file\"; filename=\"generic_pla_white.xml.fdm_material\"", ""), + call.postFormWithParts(target = "materials/", parts = ["_xXx_"], on_finished = job.sendingFinished)], + device_mock.method_calls) + + @patch("cura.Settings.CuraContainerRegistry") + @patch("cura.CuraApplication") + @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") + @patch("PyQt5.QtNetwork.QNetworkReply") + def test__onGetRemoteMaterials_withNewMaterial(self, reply_mock, device_mock, application_mock, + container_registry_mock): + application_mock.getContainerRegistry.return_value = container_registry_mock + + device_mock.createFormPart.return_value = "_xXx_" + + container_registry_mock.findContainersMetadata.return_value = [self._LOCAL_MATERIAL_WHITE, + self._LOCAL_MATERIAL_BLACK] + + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_BLACK]).encode("ascii")) + + with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) + self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) + self.assertEqual(1, device_mock.createFormPart.call_count) + self.assertEqual(1, device_mock.postFormWithParts.call_count) + self.assertEquals( + [call.createFormPart("name=\"file\"; filename=\"generic_pla_white.xml.fdm_material\"", ""), + call.postFormWithParts(target = "materials/", parts = ["_xXx_"], on_finished = job.sendingFinished)], + device_mock.method_calls) From f3338aa187bfe607664d0d79d5848d7cdee5fc06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Tue, 20 Nov 2018 16:53:01 +0100 Subject: [PATCH 05/11] Fixed the failing tests --- plugins/UM3NetworkPrinting/src/SendMaterialJob.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 0599101379..fe0cd98964 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -84,8 +84,9 @@ class SendMaterialJob(Job): def _determineMaterialsToSend(local_materials: Dict[str, LocalMaterial], remote_materials: Dict[str, ClusterMaterial]) -> Set[str]: return { - material.id for guid, material in local_materials.items() - if guid not in remote_materials or material.version > remote_materials[guid].version + material.id + for guid, material in local_materials.items() + if guid not in remote_materials or int(material.version) > remote_materials[guid].version } ## Send the materials to the printer. From 9e8be286af8736a97bf05db3b4bb5b0063be944b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Wed, 21 Nov 2018 10:12:53 +0100 Subject: [PATCH 06/11] Used NamedTuple from typing iso namedtuple from collections so we can at least give type hints --- plugins/UM3NetworkPrinting/src/Models.py | 54 +++++++++---------- .../UM3NetworkPrinting/src/SendMaterialJob.py | 51 +++++++++--------- .../tests/TestSendMaterialJob.py | 4 +- 3 files changed, 55 insertions(+), 54 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index a9210ac5b4..e84a39db5a 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -1,33 +1,33 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from collections import namedtuple +from typing import NamedTuple -ClusterMaterial = namedtuple('ClusterMaterial', [ - 'guid', - 'material', - 'brand', - 'version', - 'color', - 'density' +ClusterMaterial = NamedTuple("ClusterMaterial", [ + ("guid", str), + ("material", str), + ("brand", str), + ("version", int), + ("color", str), + ("density", str), ]) -LocalMaterial = namedtuple('LocalMaterial', [ - 'GUID', - 'id', - 'type', - 'status', - 'base_file', - 'setting_version', - 'version', - 'name', - 'brand', - 'material', - 'color_name', - 'color_code', - 'description', - 'adhesion_info', - 'approximate_diameter', - 'properties', - 'definition', - 'compatible' +LocalMaterial = NamedTuple("LocalMaterial", [ + ("GUID", str), + ("id", str), + ("type", str), + ("status", str), + ("base_file", str), + ("setting_version", int), + ("version", int), + ("name", str), + ("brand", str), + ("material", str), + ("color_name", str), + ("color_code", str), + ("description", str), + ("adhesion_info", str), + ("approximate_diameter", str), + ("properties", str), + ("definition", str), + ("compatible", str), ]) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index fe0cd98964..ee8dd8042d 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -2,7 +2,6 @@ # Cura is released under the terms of the LGPLv3 or higher. import json import os -import re import urllib.parse from typing import Dict, TYPE_CHECKING, Set @@ -13,13 +12,13 @@ from UM.Logger import Logger from UM.MimeTypeDatabase import MimeTypeDatabase from UM.Resources import Resources from cura.CuraApplication import CuraApplication - # Absolute imports don't work in plugins from .Models import ClusterMaterial, LocalMaterial if TYPE_CHECKING: from .ClusterUM3OutputDevice import ClusterUM3OutputDevice + ## Asynchronous job to send material profiles to the printer. # # This way it won't freeze up the interface while sending those materials. @@ -86,7 +85,7 @@ class SendMaterialJob(Job): return { material.id for guid, material in local_materials.items() - if guid not in remote_materials or int(material.version) > remote_materials[guid].version + if guid not in remote_materials or material.version > remote_materials[guid].version } ## Send the materials to the printer. @@ -122,23 +121,23 @@ class SendMaterialJob(Job): # \param material_id The ID of the material in the file. def _sendMaterialFile(self, file_path: str, file_name: str, material_id: str) -> None: - parts = [] + parts = [] - # Add the material file. - with open(file_path, "rb") as f: - parts.append(self.device.createFormPart("name=\"file\"; filename=\"{file_name}\"" - .format(file_name = file_name), f.read())) + # Add the material file. + with open(file_path, "rb") as f: + parts.append(self.device.createFormPart("name=\"file\"; filename=\"{file_name}\"" + .format(file_name = file_name), f.read())) - # Add the material signature file if needed. - signature_file_path = "{}.sig".format(file_path) - if os.path.exists(signature_file_path): - signature_file_name = os.path.basename(signature_file_path) - with open(signature_file_path, "rb") as f: - parts.append(self.device.createFormPart("name=\"signature_file\"; filename=\"{file_name}\"" - .format(file_name = signature_file_name), f.read())) + # Add the material signature file if needed. + signature_file_path = "{}.sig".format(file_path) + if os.path.exists(signature_file_path): + signature_file_name = os.path.basename(signature_file_path) + with open(signature_file_path, "rb") as f: + parts.append(self.device.createFormPart("name=\"signature_file\"; filename=\"{file_name}\"" + .format(file_name = signature_file_name), f.read())) - Logger.log("d", "Syncing material {material_id} with cluster.".format(material_id = material_id)) - self.device.postFormWithParts(target = "materials/", parts = parts, on_finished = self.sendingFinished) + Logger.log("d", "Syncing material {material_id} with cluster.".format(material_id = material_id)) + self.device.postFormWithParts(target = "materials/", parts = parts, on_finished = self.sendingFinished) ## Check a reply from an upload to the printer and log an error when the call failed @staticmethod @@ -174,16 +173,18 @@ class SendMaterialJob(Job): # Find the latest version of all material containers in the registry. for material in material_containers: try: - material = LocalMaterial(**material) - # material version must be an int - if not re.match("\d+", material.version): - Logger.logException("w", "Local material {} has invalid version '{}'." - .format(material["id"], material.version)) - continue + material["version"] = int(material["version"]) - if material.GUID not in result or material.version > result.get(material.GUID).version: - result[material.GUID] = material + # Create a new local material + local_material = LocalMaterial(**material) + + if local_material.GUID not in result or \ + local_material.version > result.get(local_material.GUID).version: + result[local_material.GUID] = local_material + + except KeyError: + Logger.logException("w", "Local material {} has missing values.".format(material["id"])) except ValueError: Logger.logException("w", "Local material {} has invalid values.".format(material["id"])) diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index f4604580fe..ff896683e1 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -19,7 +19,7 @@ from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: ["/materials/generic_pla_white.xml.fdm_material"]) class TestSendMaterialJob(TestCase): _LOCAL_MATERIAL_WHITE = {"type": "material", "status": "unknown", "id": "generic_pla_white", - "base_file": "generic_pla_white", "setting_version": 5, "name": "White PLA", + "base_file": "generic_pla_white", "setting_version": "5", "name": "White PLA", "brand": "Generic", "material": "PLA", "color_name": "White", "GUID": "badb0ee7-87c8-4f3f-9398-938587b67dce", "version": "1", "color_code": "#ffffff", "description": "Test PLA White", "adhesion_info": "Use glue.", "approximate_diameter": "3", @@ -27,7 +27,7 @@ class TestSendMaterialJob(TestCase): "definition": "fdmprinter", "compatible": True} _LOCAL_MATERIAL_BLACK = {"type": "material", "status": "unknown", "id": "generic_pla_black", - "base_file": "generic_pla_black", "setting_version": 5, "name": "Yellow CPE", + "base_file": "generic_pla_black", "setting_version": "5", "name": "Yellow CPE", "brand": "Ultimaker", "material": "CPE", "color_name": "Black", "GUID": "5fbb362a-41f9-4818-bb43-15ea6df34aa4", "version": "1", "color_code": "#000000", "description": "Test PLA Black", "adhesion_info": "Use glue.", "approximate_diameter": "3", From 7b0f8882a2715d4beb9378646bc915e77c8f4963 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Wed, 21 Nov 2018 11:01:26 +0100 Subject: [PATCH 07/11] Reverted models to namedtuples from collections because NamedTuple is a Python3.6 feature --- plugins/UM3NetworkPrinting/src/Models.py | 54 ++++++++++++------------ 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index e84a39db5a..d5e1007555 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -1,33 +1,33 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from typing import NamedTuple +from collections import namedtuple -ClusterMaterial = NamedTuple("ClusterMaterial", [ - ("guid", str), - ("material", str), - ("brand", str), - ("version", int), - ("color", str), - ("density", str), +ClusterMaterial = namedtuple('ClusterMaterial', [ + 'guid', # Type: str + 'material', # Type: str + 'brand', # Type: str + 'version', # Type: int + 'color', # Type: str + 'density' # Type: str ]) -LocalMaterial = NamedTuple("LocalMaterial", [ - ("GUID", str), - ("id", str), - ("type", str), - ("status", str), - ("base_file", str), - ("setting_version", int), - ("version", int), - ("name", str), - ("brand", str), - ("material", str), - ("color_name", str), - ("color_code", str), - ("description", str), - ("adhesion_info", str), - ("approximate_diameter", str), - ("properties", str), - ("definition", str), - ("compatible", str), +LocalMaterial = namedtuple('LocalMaterial', [ + 'GUID', # Type: str + 'id', # Type: str + 'type', # Type: str + 'status', # Type: str + 'base_file', # Type: str + 'setting_version', # Type: int + 'version', # Type: int + 'name', # Type: str + 'brand', # Type: str + 'material', # Type: str + 'color_name', # Type: str + 'color_code', # Type: str + 'description', # Type: str + 'adhesion_info', # Type: str + 'approximate_diameter', # Type: str + 'properties', # Type: str + 'definition', # Type: str + 'compatible' # Type: str ]) From 7e3f86f0913b7432c46b20507e33be57b7a1d514 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Thu, 22 Nov 2018 09:37:47 +0100 Subject: [PATCH 08/11] Moved some of the mocks to class level because they are used in every test method --- .../tests/TestSendMaterialJob.py | 35 ++++++------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index ff896683e1..548704fd33 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -17,6 +17,8 @@ from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob lambda _: MimeType(name = "application/x-ultimaker-material-profile", comment = "Ultimaker Material Profile", suffixes = ["xml.fdm_material"])) @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: ["/materials/generic_pla_white.xml.fdm_material"]) +@patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") +@patch("PyQt5.QtNetwork.QNetworkReply") class TestSendMaterialJob(TestCase): _LOCAL_MATERIAL_WHITE = {"type": "material", "status": "unknown", "id": "generic_pla_white", "base_file": "generic_pla_white", "setting_version": "5", "name": "White PLA", @@ -52,16 +54,13 @@ class TestSendMaterialJob(TestCase): "density": 1.00 } - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - def test_run(self, device_mock): + def test_run(self, device_mock, reply_mock): job = SendMaterialJob(device_mock) job.run() # We expect the materials endpoint to be called when the job runs. device_mock.get.assert_called_with("materials/", on_finished = job._onGetRemoteMaterials) - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") def test__onGetRemoteMaterials_withFailedRequest(self, reply_mock, device_mock): reply_mock.attribute.return_value = 404 job = SendMaterialJob(device_mock) @@ -71,8 +70,6 @@ class TestSendMaterialJob(TestCase): self.assertEqual([call.attribute(0), call.errorString()], reply_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") def test__onGetRemoteMaterials_withBadJsonAnswer(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(b"Six sick hicks nick six slick bricks with picks and sticks.") @@ -84,8 +81,6 @@ class TestSendMaterialJob(TestCase): self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") def test__onGetRemoteMaterials_withMissingGuidInRemoteMaterial(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 remote_material_without_guid = self._REMOTE_MATERIAL_WHITE.copy() @@ -101,10 +96,8 @@ class TestSendMaterialJob(TestCase): @patch("cura.Settings.CuraContainerRegistry") @patch("cura.CuraApplication") - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") - def test__onGetRemoteMaterials_withInvalidVersionInLocalMaterial(self, reply_mock, device_mock, application_mock, - container_registry_mock): + def test__onGetRemoteMaterials_withInvalidVersionInLocalMaterial(self, application_mock, container_registry_mock, + reply_mock, device_mock): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii")) @@ -125,10 +118,8 @@ class TestSendMaterialJob(TestCase): @patch("cura.Settings.CuraContainerRegistry") @patch("cura.CuraApplication") - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") - def test__onGetRemoteMaterials_withNoUpdate(self, reply_mock, device_mock, application_mock, - container_registry_mock): + def test__onGetRemoteMaterials_withNoUpdate(self, application_mock, container_registry_mock, reply_mock, + device_mock): application_mock.getContainerRegistry.return_value = container_registry_mock device_mock.createFormPart.return_value = "_xXx_" @@ -150,10 +141,8 @@ class TestSendMaterialJob(TestCase): @patch("cura.Settings.CuraContainerRegistry") @patch("cura.CuraApplication") - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") - def test__onGetRemoteMaterials_withUpdatedMaterial(self, reply_mock, device_mock, application_mock, - container_registry_mock): + def test__onGetRemoteMaterials_withUpdatedMaterial(self, application_mock, container_registry_mock, reply_mock, + device_mock): application_mock.getContainerRegistry.return_value = container_registry_mock device_mock.createFormPart.return_value = "_xXx_" @@ -181,10 +170,8 @@ class TestSendMaterialJob(TestCase): @patch("cura.Settings.CuraContainerRegistry") @patch("cura.CuraApplication") - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") - def test__onGetRemoteMaterials_withNewMaterial(self, reply_mock, device_mock, application_mock, - container_registry_mock): + def test__onGetRemoteMaterials_withNewMaterial(self, application_mock, container_registry_mock, reply_mock, + device_mock): application_mock.getContainerRegistry.return_value = container_registry_mock device_mock.createFormPart.return_value = "_xXx_" From 352427e4601204851daba848599153ebab3f8c41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Thu, 22 Nov 2018 10:01:15 +0100 Subject: [PATCH 09/11] Moved exception handling closer to the cause of error --- plugins/UM3NetworkPrinting/src/SendMaterialJob.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index ee8dd8042d..72269040e7 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -46,9 +46,8 @@ class SendMaterialJob(Job): # Collect materials from the printer's reply and send the missing ones if needed. try: remote_materials_by_guid = self._parseReply(reply) - self._sendMissingMaterials(remote_materials_by_guid) - except json.JSONDecodeError: - Logger.logException("w", "Error parsing materials from printer") + if remote_materials_by_guid: + self._sendMissingMaterials(remote_materials_by_guid) except TypeError: Logger.logException("w", "Error parsing materials from printer") @@ -153,12 +152,14 @@ class SendMaterialJob(Job): # Parses the reply to a "/materials" request to the printer # # \return a dictionary of ClusterMaterial objects by GUID - # \throw json.JSONDecodeError Raised when the reply does not contain a valid json string # \throw KeyError Raised when on of the materials does not include a valid guid @classmethod def _parseReply(cls, reply: QNetworkReply) -> Dict[str, ClusterMaterial]: - remote_materials = json.loads(reply.readAll().data().decode("utf-8")) - return {material["guid"]: ClusterMaterial(**material) for material in remote_materials} + try: + remote_materials = json.loads(reply.readAll().data().decode("utf-8")) + return {material["guid"]: ClusterMaterial(**material) for material in remote_materials} + except json.JSONDecodeError: + Logger.logException("w", "Error parsing materials from printer") ## Retrieves a list of local materials # From bb5c0326de589ee09b2398b74a542f838d6ea8e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 23 Nov 2018 09:20:19 +0100 Subject: [PATCH 10/11] Used duoble quotes iso single quotes --- plugins/UM3NetworkPrinting/src/Models.py | 52 ++++++++++++------------ 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index d5e1007555..bcdeb8299c 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -2,32 +2,32 @@ # Cura is released under the terms of the LGPLv3 or higher. from collections import namedtuple -ClusterMaterial = namedtuple('ClusterMaterial', [ - 'guid', # Type: str - 'material', # Type: str - 'brand', # Type: str - 'version', # Type: int - 'color', # Type: str - 'density' # Type: str +ClusterMaterial = namedtuple("ClusterMaterial", [ + "guid", # Type: str + "material", # Type: str + "brand", # Type: str + "version", # Type: int + "color", # Type: str + "density" # Type: str ]) -LocalMaterial = namedtuple('LocalMaterial', [ - 'GUID', # Type: str - 'id', # Type: str - 'type', # Type: str - 'status', # Type: str - 'base_file', # Type: str - 'setting_version', # Type: int - 'version', # Type: int - 'name', # Type: str - 'brand', # Type: str - 'material', # Type: str - 'color_name', # Type: str - 'color_code', # Type: str - 'description', # Type: str - 'adhesion_info', # Type: str - 'approximate_diameter', # Type: str - 'properties', # Type: str - 'definition', # Type: str - 'compatible' # Type: str +LocalMaterial = namedtuple("LocalMaterial", [ + "GUID", # Type: str + "id", # Type: str + "type", # Type: str + "status", # Type: str + "base_file", # Type: str + "setting_version", # Type: int + "version", # Type: int + "name", # Type: str + "brand", # Type: str + "material", # Type: str + "color_name", # Type: str + "color_code", # Type: str + "description", # Type: str + "adhesion_info", # Type: str + "approximate_diameter", # Type: str + "properties", # Type: str + "definition", # Type: str + "compatible" # Type: str ]) From 294527f7febf96d81f4bcdf62b142a552d58fbbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 23 Nov 2018 09:21:09 +0100 Subject: [PATCH 11/11] Review changes --- .../UM3NetworkPrinting/src/SendMaterialJob.py | 19 ++++++------ .../tests/TestSendMaterialJob.py | 29 +++++++++++++------ 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 72269040e7..48760af28e 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -7,6 +7,7 @@ from typing import Dict, TYPE_CHECKING, Set from PyQt5.QtNetwork import QNetworkReply, QNetworkRequest +from UM.Application import Application from UM.Job import Job from UM.Logger import Logger from UM.MimeTypeDatabase import MimeTypeDatabase @@ -27,7 +28,6 @@ class SendMaterialJob(Job): def __init__(self, device: "ClusterUM3OutputDevice") -> None: super().__init__() self.device = device # type: ClusterUM3OutputDevice - self._application = CuraApplication.getInstance() # type: CuraApplication ## Send the request to the printer and register a callback def run(self) -> None: @@ -44,12 +44,9 @@ class SendMaterialJob(Job): return # Collect materials from the printer's reply and send the missing ones if needed. - try: - remote_materials_by_guid = self._parseReply(reply) - if remote_materials_by_guid: - self._sendMissingMaterials(remote_materials_by_guid) - except TypeError: - Logger.logException("w", "Error parsing materials from printer") + remote_materials_by_guid = self._parseReply(reply) + if remote_materials_by_guid: + self._sendMissingMaterials(remote_materials_by_guid) ## Determine which materials should be updated and send them to the printer. # @@ -158,8 +155,12 @@ class SendMaterialJob(Job): try: remote_materials = json.loads(reply.readAll().data().decode("utf-8")) return {material["guid"]: ClusterMaterial(**material) for material in remote_materials} + except UnicodeDecodeError: + Logger.log("e", "Request material storage on printer: I didn't understand the printer's answer.") except json.JSONDecodeError: - Logger.logException("w", "Error parsing materials from printer") + Logger.log("e", "Request material storage on printer: I didn't understand the printer's answer.") + except TypeError: + Logger.log("e", "Request material storage on printer: Printer's answer was missing GUIDs.") ## Retrieves a list of local materials # @@ -168,7 +169,7 @@ class SendMaterialJob(Job): # \return a dictionary of LocalMaterial objects by GUID def _getLocalMaterials(self) -> Dict[str, LocalMaterial]: result = {} # type: Dict[str, LocalMaterial] - container_registry = self._application.getContainerRegistry() + container_registry = Application.getInstance().getContainerRegistry() material_containers = container_registry.findContainersMetadata(type = "material") # Find the latest version of all material containers in the registry. diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index 548704fd33..f5a475b3ab 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -8,7 +8,7 @@ from unittest.mock import patch, call from PyQt5.QtCore import QByteArray from UM.MimeTypeDatabase import MimeType -from cura.CuraApplication import CuraApplication +from UM.Application import Application from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob @@ -70,6 +70,17 @@ class TestSendMaterialJob(TestCase): self.assertEqual([call.attribute(0), call.errorString()], reply_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) + def test__onGetRemoteMaterials_withWrongEncoding(self, reply_mock, device_mock): + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("cp500")) + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + # We expect the reply to be called once to try to get the printers from the list (readAll()). + # Given that the parsing fails we do no expect the device to be called for any follow up. + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual(0, device_mock.createFormPart.call_count) + def test__onGetRemoteMaterials_withBadJsonAnswer(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(b"Six sick hicks nick six slick bricks with picks and sticks.") @@ -95,7 +106,7 @@ class TestSendMaterialJob(TestCase): self.assertEqual(0, device_mock.createFormPart.call_count) @patch("cura.Settings.CuraContainerRegistry") - @patch("cura.CuraApplication") + @patch("UM.Application") def test__onGetRemoteMaterials_withInvalidVersionInLocalMaterial(self, application_mock, container_registry_mock, reply_mock, device_mock): reply_mock.attribute.return_value = 200 @@ -107,7 +118,7 @@ class TestSendMaterialJob(TestCase): application_mock.getContainerRegistry.return_value = container_registry_mock - with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + with mock.patch.object(Application, "getInstance", new = lambda: application_mock): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) @@ -117,7 +128,7 @@ class TestSendMaterialJob(TestCase): self.assertEqual(0, device_mock.createFormPart.call_count) @patch("cura.Settings.CuraContainerRegistry") - @patch("cura.CuraApplication") + @patch("UM.Application") def test__onGetRemoteMaterials_withNoUpdate(self, application_mock, container_registry_mock, reply_mock, device_mock): application_mock.getContainerRegistry.return_value = container_registry_mock @@ -129,7 +140,7 @@ class TestSendMaterialJob(TestCase): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii")) - with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + with mock.patch.object(Application, "getInstance", new = lambda: application_mock): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) @@ -140,7 +151,7 @@ class TestSendMaterialJob(TestCase): self.assertEqual(0, device_mock.postFormWithParts.call_count) @patch("cura.Settings.CuraContainerRegistry") - @patch("cura.CuraApplication") + @patch("UM.Application") def test__onGetRemoteMaterials_withUpdatedMaterial(self, application_mock, container_registry_mock, reply_mock, device_mock): application_mock.getContainerRegistry.return_value = container_registry_mock @@ -154,7 +165,7 @@ class TestSendMaterialJob(TestCase): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii")) - with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + with mock.patch.object(Application, "getInstance", new = lambda: application_mock): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) @@ -169,7 +180,7 @@ class TestSendMaterialJob(TestCase): device_mock.method_calls) @patch("cura.Settings.CuraContainerRegistry") - @patch("cura.CuraApplication") + @patch("UM.Application") def test__onGetRemoteMaterials_withNewMaterial(self, application_mock, container_registry_mock, reply_mock, device_mock): application_mock.getContainerRegistry.return_value = container_registry_mock @@ -182,7 +193,7 @@ class TestSendMaterialJob(TestCase): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_BLACK]).encode("ascii")) - with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + with mock.patch.object(Application, "getInstance", new = lambda: application_mock): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock)