Copilot commented on code in PR #1084: URL: https://github.com/apache/mahout/pull/1084#discussion_r2844263912
########## testing/qumat/test_qdp_module.py: ########## @@ -0,0 +1,70 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys +import importlib +import pytest +from unittest.mock import patch + + +def _reload_qdp_without_extension(): + """ + Safely reload qumat.qdp while simulating missing _qdp extension. + """ + # Remove cached modules + sys.modules.pop("qumat.qdp", None) + sys.modules.pop("_qdp", None) + + # Simulate missing compiled extension + with patch.dict(sys.modules, {"_qdp": None}): + import qumat.qdp + return importlib.reload(qumat.qdp) + + +def test_qdp_import_fallback_warning(): + with pytest.warns(ImportWarning): + qdp = _reload_qdp_without_extension() + assert qdp is not None + + +def test_qdp_engine_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QdpEngine() + + +def test_quantum_tensor_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QuantumTensor() + + +def test_qdp_all_exports(): + qdp = _reload_qdp_without_extension() + + assert "QdpEngine" in qdp.__all__ + assert "QuantumTensor" in qdp.__all__ + + +def test_make_stub_direct_call(): + qdp = _reload_qdp_without_extension() Review Comment: The warning emitted during module reload should be captured to prevent test pollution. When _reload_qdp_without_extension is called, it triggers an ImportWarning (line 53-58 in qumat/qdp.py), but unlike test_qdp_import_fallback_warning, this test doesn't capture it with pytest.warns(). This can cause warnings to leak into the test output and potentially interfere with other tests that check for warnings. ```suggestion with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() with pytest.raises(ImportError): qdp.QdpEngine() def test_quantum_tensor_stub_raises_import_error(): with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() with pytest.raises(ImportError): qdp.QuantumTensor() def test_qdp_all_exports(): with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() assert "QdpEngine" in qdp.__all__ assert "QuantumTensor" in qdp.__all__ def test_make_stub_direct_call(): with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() ``` ########## testing/qumat/test_qdp_module.py: ########## @@ -0,0 +1,70 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys +import importlib +import pytest +from unittest.mock import patch + + +def _reload_qdp_without_extension(): + """ + Safely reload qumat.qdp while simulating missing _qdp extension. + """ + # Remove cached modules + sys.modules.pop("qumat.qdp", None) + sys.modules.pop("_qdp", None) + + # Simulate missing compiled extension + with patch.dict(sys.modules, {"_qdp": None}): + import qumat.qdp + return importlib.reload(qumat.qdp) + + +def test_qdp_import_fallback_warning(): + with pytest.warns(ImportWarning): + qdp = _reload_qdp_without_extension() + assert qdp is not None + + +def test_qdp_engine_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QdpEngine() + + +def test_quantum_tensor_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QuantumTensor() + + +def test_qdp_all_exports(): + qdp = _reload_qdp_without_extension() + + assert "QdpEngine" in qdp.__all__ + assert "QuantumTensor" in qdp.__all__ + + +def test_make_stub_direct_call(): + qdp = _reload_qdp_without_extension() + + StubClass = qdp._make_stub("TestStub") + + with pytest.raises(ImportError): + StubClass() Review Comment: The warning emitted during module reload should be captured to prevent test pollution. When _reload_qdp_without_extension is called, it triggers an ImportWarning (line 53-58 in qumat/qdp.py), but unlike test_qdp_import_fallback_warning, this test doesn't capture it with pytest.warns(). This can cause warnings to leak into the test output and potentially interfere with other tests that check for warnings. ########## testing/qumat/test_qdp_module.py: ########## @@ -0,0 +1,70 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys +import importlib +import pytest +from unittest.mock import patch + + +def _reload_qdp_without_extension(): + """ + Safely reload qumat.qdp while simulating missing _qdp extension. + """ + # Remove cached modules + sys.modules.pop("qumat.qdp", None) + sys.modules.pop("_qdp", None) + + # Simulate missing compiled extension + with patch.dict(sys.modules, {"_qdp": None}): + import qumat.qdp + return importlib.reload(qumat.qdp) + + +def test_qdp_import_fallback_warning(): + with pytest.warns(ImportWarning): + qdp = _reload_qdp_without_extension() + assert qdp is not None + + +def test_qdp_engine_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QdpEngine() + + +def test_quantum_tensor_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QuantumTensor() + + +def test_qdp_all_exports(): + qdp = _reload_qdp_without_extension() + + assert "QdpEngine" in qdp.__all__ + assert "QuantumTensor" in qdp.__all__ + + +def test_make_stub_direct_call(): + qdp = _reload_qdp_without_extension() Review Comment: The warning emitted during module reload should be captured to prevent test pollution. When _reload_qdp_without_extension is called, it triggers an ImportWarning (line 53-58 in qumat/qdp.py), but unlike test_qdp_import_fallback_warning, this test doesn't capture it with pytest.warns(). This can cause warnings to leak into the test output and potentially interfere with other tests that check for warnings. ```suggestion with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() with pytest.raises(ImportError): qdp.QdpEngine() def test_quantum_tensor_stub_raises_import_error(): with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() with pytest.raises(ImportError): qdp.QuantumTensor() def test_qdp_all_exports(): with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() assert "QdpEngine" in qdp.__all__ assert "QuantumTensor" in qdp.__all__ def test_make_stub_direct_call(): with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() ``` ########## testing/qumat/test_qdp_module.py: ########## @@ -0,0 +1,70 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys +import importlib +import pytest +from unittest.mock import patch + + +def _reload_qdp_without_extension(): + """ + Safely reload qumat.qdp while simulating missing _qdp extension. + """ + # Remove cached modules + sys.modules.pop("qumat.qdp", None) + sys.modules.pop("_qdp", None) + + # Simulate missing compiled extension + with patch.dict(sys.modules, {"_qdp": None}): + import qumat.qdp + return importlib.reload(qumat.qdp) + + +def test_qdp_import_fallback_warning(): + with pytest.warns(ImportWarning): + qdp = _reload_qdp_without_extension() + assert qdp is not None + + +def test_qdp_engine_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QdpEngine() + + +def test_quantum_tensor_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QuantumTensor() + + +def test_qdp_all_exports(): + qdp = _reload_qdp_without_extension() + + assert "QdpEngine" in qdp.__all__ + assert "QuantumTensor" in qdp.__all__ + + +def test_make_stub_direct_call(): + qdp = _reload_qdp_without_extension() + + StubClass = qdp._make_stub("TestStub") + + with pytest.raises(ImportError): + StubClass() Review Comment: Consider adding a match parameter to verify the specific error message. Other tests in the codebase (e.g., testing/qumat/test_create_circuit.py:181, testing/qdp/test_bindings.py:120) use pytest.raises with match to ensure the correct error is raised. This would verify that the stub raises the expected ImportError with the installation message from _INSTALL_MSG. ########## testing/qumat/test_qdp_module.py: ########## @@ -0,0 +1,70 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys +import importlib +import pytest +from unittest.mock import patch + + +def _reload_qdp_without_extension(): + """ + Safely reload qumat.qdp while simulating missing _qdp extension. + """ + # Remove cached modules + sys.modules.pop("qumat.qdp", None) + sys.modules.pop("_qdp", None) + + # Simulate missing compiled extension + with patch.dict(sys.modules, {"_qdp": None}): + import qumat.qdp + return importlib.reload(qumat.qdp) + + +def test_qdp_import_fallback_warning(): + with pytest.warns(ImportWarning): + qdp = _reload_qdp_without_extension() + assert qdp is not None + + +def test_qdp_engine_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QdpEngine() + + +def test_quantum_tensor_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QuantumTensor() + + +def test_qdp_all_exports(): + qdp = _reload_qdp_without_extension() + + assert "QdpEngine" in qdp.__all__ + assert "QuantumTensor" in qdp.__all__ + + +def test_make_stub_direct_call(): + qdp = _reload_qdp_without_extension() Review Comment: The warning emitted during module reload should be captured to prevent test pollution. When _reload_qdp_without_extension is called, it triggers an ImportWarning (line 53-58 in qumat/qdp.py), but unlike test_qdp_import_fallback_warning, this test doesn't capture it with pytest.warns(). This can cause warnings to leak into the test output and potentially interfere with other tests that check for warnings. ```suggestion with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() with pytest.raises(ImportError): qdp.QdpEngine() def test_quantum_tensor_stub_raises_import_error(): with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() with pytest.raises(ImportError): qdp.QuantumTensor() def test_qdp_all_exports(): with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() assert "QdpEngine" in qdp.__all__ assert "QuantumTensor" in qdp.__all__ def test_make_stub_direct_call(): with pytest.warns(ImportWarning): qdp = _reload_qdp_without_extension() ``` ########## testing/qumat/test_qdp_module.py: ########## @@ -0,0 +1,70 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys +import importlib +import pytest +from unittest.mock import patch + + +def _reload_qdp_without_extension(): + """ + Safely reload qumat.qdp while simulating missing _qdp extension. + """ + # Remove cached modules + sys.modules.pop("qumat.qdp", None) + sys.modules.pop("_qdp", None) + + # Simulate missing compiled extension + with patch.dict(sys.modules, {"_qdp": None}): + import qumat.qdp + return importlib.reload(qumat.qdp) Review Comment: The import on line 33 appears redundant. After removing qumat.qdp from sys.modules on line 28, importlib.reload on line 34 will reimport the module. The import statement on line 33 could be removed, and the code would function identically. If the import is intentional for clarity, consider adding a comment explaining why both import and reload are needed. ```suggestion module = importlib.import_module("qumat.qdp") return importlib.reload(module) ``` ########## testing/qumat/test_qdp_module.py: ########## @@ -0,0 +1,70 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys +import importlib +import pytest +from unittest.mock import patch + + +def _reload_qdp_without_extension(): + """ + Safely reload qumat.qdp while simulating missing _qdp extension. + """ + # Remove cached modules + sys.modules.pop("qumat.qdp", None) + sys.modules.pop("_qdp", None) + + # Simulate missing compiled extension + with patch.dict(sys.modules, {"_qdp": None}): + import qumat.qdp + return importlib.reload(qumat.qdp) + + +def test_qdp_import_fallback_warning(): + with pytest.warns(ImportWarning): + qdp = _reload_qdp_without_extension() + assert qdp is not None + + +def test_qdp_engine_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QdpEngine() Review Comment: Consider adding a match parameter to verify the specific error message. Other tests in the codebase (e.g., testing/qumat/test_create_circuit.py:181, testing/qdp/test_bindings.py:120) use pytest.raises with match to ensure the correct error is raised. This would verify that the stub raises the expected ImportError with the installation message from _INSTALL_MSG. ########## testing/qumat/test_qdp_module.py: ########## @@ -0,0 +1,70 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys +import importlib +import pytest +from unittest.mock import patch + + +def _reload_qdp_without_extension(): + """ + Safely reload qumat.qdp while simulating missing _qdp extension. + """ + # Remove cached modules + sys.modules.pop("qumat.qdp", None) + sys.modules.pop("_qdp", None) + + # Simulate missing compiled extension + with patch.dict(sys.modules, {"_qdp": None}): + import qumat.qdp + return importlib.reload(qumat.qdp) + + +def test_qdp_import_fallback_warning(): + with pytest.warns(ImportWarning): + qdp = _reload_qdp_without_extension() + assert qdp is not None + + +def test_qdp_engine_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QdpEngine() + + +def test_quantum_tensor_stub_raises_import_error(): + qdp = _reload_qdp_without_extension() + + with pytest.raises(ImportError): + qdp.QuantumTensor() Review Comment: Consider adding a match parameter to verify the specific error message. Other tests in the codebase (e.g., testing/qumat/test_create_circuit.py:181, testing/qdp/test_bindings.py:120) use pytest.raises with match to ensure the correct error is raised. This would verify that the stub raises the expected ImportError with the installation message from _INSTALL_MSG. ########## testing/qumat/test_qdp_module.py: ########## @@ -0,0 +1,70 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys +import importlib +import pytest +from unittest.mock import patch + + +def _reload_qdp_without_extension(): + """ + Safely reload qumat.qdp while simulating missing _qdp extension. + """ + # Remove cached modules + sys.modules.pop("qumat.qdp", None) + sys.modules.pop("_qdp", None) + + # Simulate missing compiled extension + with patch.dict(sys.modules, {"_qdp": None}): + import qumat.qdp + return importlib.reload(qumat.qdp) + + Review Comment: The helper function permanently removes modules from sys.modules without cleanup. If qumat.qdp or _qdp were previously imported in the test session, they are removed and not restored, which could affect other tests. Consider using a fixture with proper teardown to restore the original sys.modules state, or document that these tests should run in isolation. ```suggestion # Preserve existing modules so we can restore them after the test helper runs original_qdp = sys.modules.get("qumat.qdp") original_qdp_ext = sys.modules.get("_qdp") try: # Remove cached modules sys.modules.pop("qumat.qdp", None) sys.modules.pop("_qdp", None) # Simulate missing compiled extension with patch.dict(sys.modules, {"_qdp": None}): import qumat.qdp return importlib.reload(qumat.qdp) finally: # Restore previous sys.modules state to avoid affecting other tests if original_qdp is not None: sys.modules["qumat.qdp"] = original_qdp else: sys.modules.pop("qumat.qdp", None) if original_qdp_ext is not None: sys.modules["_qdp"] = original_qdp_ext else: sys.modules.pop("_qdp", None) ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
