From 46bb1b83ea0843c25783f86e2033a10aeaed79d2 Mon Sep 17 00:00:00 2001
From: jvoisin <julien.voisin@dustri.org>
Date: Wed, 5 Sep 2018 17:22:17 +0200
Subject: [PATCH] Improve the previous commit

---
 libmat2/office.py     |  8 +++++---
 mat2                  |  4 ++--
 tests/test_climat2.py |  4 ++--
 tests/test_policy.py  | 31 +++++++++++++++++++++++++++++++
 4 files changed, 40 insertions(+), 7 deletions(-)
 create mode 100644 tests/test_policy.py

diff --git a/libmat2/office.py b/libmat2/office.py
index e79fe58..224067c 100644
--- a/libmat2/office.py
+++ b/libmat2/office.py
@@ -84,6 +84,11 @@ class ArchiveBasedAbstractParser(abstract.AbstractParser):
 
     def remove_all(self) -> bool:
         # pylint: disable=too-many-branches
+
+        if self.unknown_member_policy not in ['omit', 'keep', 'abort']:
+            logging.error("The policy %s is invalid.", self.unknown_member_policy)
+            raise ValueError
+
         with zipfile.ZipFile(self.filename) as zin,\
              zipfile.ZipFile(self.output_filename, 'w') as zout:
 
@@ -120,9 +125,6 @@ class ArchiveBasedAbstractParser(abstract.AbstractParser):
                             logging.warning("In file %s, keeping unknown element %s (format: %s)",
                                             self.filename, item.filename, mtype)
                         else:
-                            if self.unknown_member_policy != 'abort':
-                                logging.warning("Invalid unknown_member_policy %s, " +
-                                                "treating as 'abort'", self.unknown_member_policy)
                             logging.error("In file %s, element %s's format (%s) " +
                                           "isn't supported",
                                           self.filename, item.filename, mtype)
diff --git a/mat2 b/mat2
index b45892e..c7f7a73 100755
--- a/mat2
+++ b/mat2
@@ -41,9 +41,9 @@ def create_arg_parser():
                         help='check if MAT2 has all the dependencies it needs')
     parser.add_argument('-V', '--verbose', action='store_true',
                         help='show more verbose status information')
-    parser.add_argument('-u', '--unknown-members', metavar='POLICY', default='abort',
+    parser.add_argument('-u', '--unknown-members', metavar='policy', default='abort',
                         help='how to handle unknown members of archive-style files ' +
-                        '(POLICY should be abort, omit, or keep)')
+                        '(policy should be abort, omit, or keep)')
 
 
     info = parser.add_mutually_exclusive_group()
diff --git a/tests/test_climat2.py b/tests/test_climat2.py
index 6ee84d5..9614347 100644
--- a/tests/test_climat2.py
+++ b/tests/test_climat2.py
@@ -8,13 +8,13 @@ class TestHelp(unittest.TestCase):
     def test_help(self):
         proc = subprocess.Popen(['./mat2', '--help'], stdout=subprocess.PIPE)
         stdout, _ = proc.communicate()
-        self.assertIn(b'usage: mat2 [-h] [-v] [-l] [-c] [-V] [-u POLICY] [-s | -L] [files [files ...]]',
+        self.assertIn(b'usage: mat2 [-h] [-v] [-l] [-c] [-V] [-u policy] [-s | -L] [files [files ...]]',
                       stdout)
 
     def test_no_arg(self):
         proc = subprocess.Popen(['./mat2'], stdout=subprocess.PIPE)
         stdout, _ = proc.communicate()
-        self.assertIn(b'usage: mat2 [-h] [-v] [-l] [-c] [-V] [-u POLICY] [-s | -L] [files [files ...]]',
+        self.assertIn(b'usage: mat2 [-h] [-v] [-l] [-c] [-V] [-u policy] [-s | -L] [files [files ...]]',
                       stdout)
 
 
diff --git a/tests/test_policy.py b/tests/test_policy.py
new file mode 100644
index 0000000..39282b1
--- /dev/null
+++ b/tests/test_policy.py
@@ -0,0 +1,31 @@
+#!/usr/bin/python3
+
+import unittest
+import shutil
+import os
+
+from libmat2 import office
+
+class TestPolicy(unittest.TestCase):
+    def test_policy_omit(self):
+        shutil.copy('./tests/data/embedded.docx', './tests/data/clean.docx')
+        p = office.MSOfficeParser('./tests/data/clean.docx')
+        p.unknown_member_policy = 'omit'
+        self.assertTrue(p.remove_all())
+        os.remove('./tests/data/clean.docx')
+
+    def test_policy_keep(self):
+        shutil.copy('./tests/data/embedded.docx', './tests/data/clean.docx')
+        p = office.MSOfficeParser('./tests/data/clean.docx')
+        p.unknown_member_policy = 'keep'
+        self.assertTrue(p.remove_all())
+        os.remove('./tests/data/clean.docx')
+
+    def test_policy_unknown(self):
+        shutil.copy('./tests/data/embedded.docx', './tests/data/clean.docx')
+        p = office.MSOfficeParser('./tests/data/clean.docx')
+        p.unknown_member_policy = 'unknown_policy_name_totally_invalid'
+        with self.assertRaises(ValueError):
+            p.remove_all()
+        os.remove('./tests/data/clean.docx')
+
-- 
GitLab