Merge pull request #1645 from userzimmermann/sprint/addoption-check
added check for already existing option names to OptionGroup.addoption()
This commit is contained in:
		
						commit
						f2bb3df310
					
				
							
								
								
									
										1
									
								
								AUTHORS
								
								
								
								
							
							
						
						
									
										1
									
								
								AUTHORS
								
								
								
								
							|  | @ -106,3 +106,4 @@ Tom Viner | ||||||
| Trevor Bekolay | Trevor Bekolay | ||||||
| Wouter van Ackooy | Wouter van Ackooy | ||||||
| Bernard Pratz | Bernard Pratz | ||||||
|  | Stefan Zimmermann | ||||||
|  |  | ||||||
|  | @ -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 | .. _@milliams: https://github.com/milliams | ||||||
| .. _@csaftoiu: https://github.com/csaftoiu | .. _@csaftoiu: https://github.com/csaftoiu | ||||||
| .. _@flub: https://github.com/flub | .. _@flub: https://github.com/flub | ||||||
|  | @ -161,6 +166,7 @@ | ||||||
| .. _#1628: https://github.com/pytest-dev/pytest/pull/1628 | .. _#1628: https://github.com/pytest-dev/pytest/pull/1628 | ||||||
| .. _#1629: https://github.com/pytest-dev/pytest/issues/1629 | .. _#1629: https://github.com/pytest-dev/pytest/issues/1629 | ||||||
| .. _#1633: https://github.com/pytest-dev/pytest/pull/1633 | .. _#1633: https://github.com/pytest-dev/pytest/pull/1633 | ||||||
|  | .. _#1618: https://github.com/pytest-dev/pytest/issues/1618 | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| **Bug Fixes** | **Bug Fixes** | ||||||
|  |  | ||||||
|  | @ -686,6 +686,10 @@ class OptionGroup: | ||||||
|         results in help showing '--two-words' only, but --twowords gets |         results in help showing '--two-words' only, but --twowords gets | ||||||
|         accepted **and** the automatic destination is in args.twowords |         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) |         option = Argument(*optnames, **attrs) | ||||||
|         self._addoption_instance(option, shortupper=False) |         self._addoption_instance(option, shortupper=False) | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -77,6 +77,13 @@ class TestParser: | ||||||
|         assert len(group.options) == 1 |         assert len(group.options) == 1 | ||||||
|         assert isinstance(group.options[0], parseopt.Argument) |         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): |     def test_group_shortopt_lowercase(self, parser): | ||||||
|         group = parser.getgroup("hello") |         group = parser.getgroup("hello") | ||||||
|         pytest.raises(ValueError, """ |         pytest.raises(ValueError, """ | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue