Merge pull request #1645 from userzimmermann/sprint/addoption-check

added check for already existing option names to OptionGroup.addoption()

Author: RonnyPfannschmidt

AUTHORS

@@ -106,3 +106,4 @@ Tom Viner
 Trevor Bekolay
 Wouter van Ackooy
 Bernard Pratz
+Stefan Zimmermann

CHANGELOG.rst

@@ -127,6 +127,11 @@
 
 *
 
+* ``OptionGroup.addoption()`` now checks if option names were already
+  added before, to make it easier to track down issues like `#1618`_.
+  Before, you only got exceptions later from ``argparse`` library,
+  giving no clue about the actual reason for double-added options.
+
 .. _@milliams: https://github.com/milliams
 .. _@csaftoiu: https://github.com/csaftoiu
 .. _@flub: https://github.com/flub
@@ -161,6 +166,7 @@
 .. _#1628: https://github.com/pytest-dev/pytest/pull/1628
 .. _#1629: https://github.com/pytest-dev/pytest/issues/1629
 .. _#1633: https://github.com/pytest-dev/pytest/pull/1633
+.. _#1618: https://github.com/pytest-dev/pytest/issues/1618
 
 
 **Bug Fixes**

_pytest/config.py

@@ -686,6 +686,10 @@ def addoption(self, *optnames, **attrs):
         results in help showing '--two-words' only, but --twowords gets
         accepted **and** the automatic destination is in args.twowords
         """
+        conflict = set(optnames).intersection(
+            name for opt in self.options for name in opt.names())
+        if conflict:
+            raise ValueError("option names %s already added" % conflict)
         option = Argument(*optnames, **attrs)
         self._addoption_instance(option, shortupper=False)
 

testing/test_parseopt.py

@@ -77,6 +77,13 @@ def test_group_addoption(self):
         assert len(group.options) == 1
         assert isinstance(group.options[0], parseopt.Argument)
 
+    def test_group_addoption_conflict(self):
+        group = parseopt.OptionGroup("hello again")
+        group.addoption("--option1", "--option-1", action="store_true")
+        with pytest.raises(ValueError) as err:
+            group.addoption("--option1", "--option-one", action="store_true")
+        assert str(set(["--option1"])) in str(err.value)
+
     def test_group_shortopt_lowercase(self, parser):
         group = parser.getgroup("hello")
         pytest.raises(ValueError, """