fix error handling in savefile_open and tests

qutebrowser/utils/qtutils.py

@@ -197,21 +197,29 @@ def deserialize_stream(stream, obj):
 def savefile_open(filename, binary=False, encoding='utf-8'):
     """Context manager to easily use a QSaveFile."""
     f = QSaveFile(filename)
+    open_ok = False
+    caller_finished = False
     cancelled = False
     try:
-        ok = f.open(QIODevice.WriteOnly)
-        if not ok:
+        open_ok = f.open(QIODevice.WriteOnly)
+        if not open_ok:
             raise QtOSError(f)
         if binary:
             new_f = PyQIODevice(f)
         else:
             new_f = io.TextIOWrapper(PyQIODevice(f), encoding=encoding)
         yield new_f
+        caller_finished = True
         new_f.flush()
     except:
         f.cancelWriting()
         cancelled = True
-        raise
+        if not open_ok:
+            raise QtOSError(f, msg="Open failed!")
+        elif not caller_finished:
+            raise
+        else:
+            raise QtOSError(f, msg="Flush failed!")
     finally:
         commit_ok = f.commit()
         if not commit_ok and not cancelled:

tests/unit/utils/test_qtutils.py

@@ -388,7 +388,6 @@ class TestSavefileOpen:
     def test_mock_open_error(self, qsavefile_mock):
         """Test with a mock and a failing open()."""
         qsavefile_mock.open.return_value = False
-        qsavefile_mock.errorString.return_value = "Hello World"
 
         with pytest.raises(OSError) as excinfo:
             with qtutils.savefile_open('filename'):
@@ -396,7 +395,7 @@ class TestSavefileOpen:
 
         qsavefile_mock.open.assert_called_once_with(QIODevice.WriteOnly)
         qsavefile_mock.cancelWriting.assert_called_once_with()
-        assert str(excinfo.value) == "Hello World"
+        assert str(excinfo.value) == "Open failed!"
 
     def test_mock_exception(self, qsavefile_mock):
         """Test with a mock and an exception in the block."""
@@ -477,20 +476,32 @@ class TestSavefileOpen:
             with qtutils.savefile_open(str(filename)):
                 pass
         errors = ["Filename refers to a directory",  # Qt >= 5.4
+                  "Open failed!",
                   "Commit failed!"]  # older Qt versions
         assert str(excinfo.value) in errors
         assert tmpdir.listdir() == [filename]
 
+    def test_failing_flush(self, tmpdir):
+        """Test with the file being closed before flushing."""
+        filename = tmpdir / 'foo'
+        with pytest.raises(OSError) as excinfo:
+            with qtutils.savefile_open(str(filename), binary=True) as f:
+                f.write(b'Hello')
+                f.dev.commit()  # provoke failing flush
+
+        assert str(excinfo.value) == "Flush failed!"
+        assert tmpdir.listdir() == [filename]
+
     def test_failing_commit(self, tmpdir):
         """Test with the file being closed before committing."""
         filename = tmpdir / 'foo'
         with pytest.raises(OSError) as excinfo:
             with qtutils.savefile_open(str(filename), binary=True) as f:
                 f.write(b'Hello')
-                f.dev.commit()  # provoke failing "real" commit
+                f.dev.cancelWriting()  # provoke failing commit
 
         assert str(excinfo.value) == "Commit failed!"
-        assert tmpdir.listdir() == [filename]
+        assert tmpdir.listdir() == []
 
     def test_line_endings(self, tmpdir):
         """Make sure line endings are translated correctly.