8000 WIP #25825: Add a failing test to demonstrate bug introduced in ##24987. · symfony/symfony@5ebc8a4 · GitHub
[go: up one dir, main page]

Skip to content

Commit 5ebc8a4

Browse files
WIP #25825: Add a failing test to demonstrate bug introduced in ##24987.
1 parent 2aa54b8 commit 5ebc8a4

File tree

1 file changed

+48
-5
lines changed

1 file changed

+48
-5
lines changed

src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -309,16 +309,59 @@ public function testGetFirstArgument()
309309
$this->assertEquals('foo', $input->getFirstArgument(), '->getFirstArgument() returns the first argument from the raw input');
310310
}
311311

312-
public function testHasParameterOption()
312+
public function testFailingHasParameterOptionTest1()
313313
{
314-
$input = new ArgvInput(array('cli.php', '-f', 'foo'));
315-
$this->assertTrue($input->hasParameterOption('-f'), '->hasParameterOption() returns true if the given short option is in the raw input');
314+
$input = new ArgvInput(array('cli.php', '-etest'));
315+
316+
// CONTROL: We should not be calling 'bind' in the hasParameterOption test;
317+
// this is here temporarily to demonstrate that -etest is interpreted as --example=test
318+
$input->bind(new InputDefinition(array(new InputOption('example', 'e', InputOption::VALUE_REQUIRED))));
319+
// [A]
320+
$this->assertEquals(array('example' => 'test'), $input->getOptions(), 'CONTROL: the short option "e" has value "test".');
321+
322+
// [B] Failing test: $input has '-etest', which is the same as --example=test,
323+
// but 'hasParameterOption' thinks that there is a '-s', because it interprets
324+
// the short option as the combination of '-e -t -e -s -t'. This is incorrect,
325+
// but hasParameterOption cannot use the InputDefinition, so it does not know this.
326+
$this->assertFalse($input->hasParameterOption('-s'), '->hasParameterOption() returns true if the given short option is in the raw input');
327+
}
316328

329+
public function testFailingHasParameterOptionTest2()
330+
{
317331
$input = new ArgvInput(array('cli.php', '-fh'));
332+
// [C] Historically, the test below was not supported. This is the use-case
333+
// that #24987 aimed to fix.
334+
$this->assertTrue($input->hasParameterOption('-f'), '->hasParameterOption() returns true if the given short option is in the raw input');
335+
// [D] Another example of a use-case that #24987 aimed to fix. However, the
336+
// implemented solution is not accurate, because hasParameterOption does
337+
// not know if the previous short option, -f, takes a value or not. If
338+
// -f takes a value, then -fh does NOT include -h. Otherwise it does.
339+
$this->assertTrue($input->hasParameterOption('-h'), '->hasParameterOption() returns true if the given short option is in the raw input');
340+
// [E] Historically, this test would pass; however, it is a bit odd. Does
341+
// it mean "commandline contains both -f AND -h"? If so, should this
342+
// also pass for 'cli.php -f -h'?
343+
// It seems like it would be reasonable to not support this use-case,
344+
// and require folks to use $input->hasParameterOption('-f') && $input->hasParameterOption('-h')
318345
$this->assertTrue($input->hasParameterOption('-fh'), '->hasParameterOption() returns true if the given short option is in the raw input');
346+
// [F] Failing test: If -fh is supported, then -hf should also work.
347+
// It seems reasonable to not support this (or '-fh').
348+
$this->assertTrue($input->hasParameterOption('-hf'), '->hasParameterOption() returns true if the given short option is in the raw input');
349+
}
319350

320-
$input = new ArgvInput(array('cli.php', '-e=test'));
321-
$this->assertFalse($input->hasParameterOption('-s'), '->hasParameterOption() returns true if the given short option is in the raw input');
351+
public function testFailingHasParameterOptionTest3()
352+
{
353+
$input = new ArgvInput(array('cli.php', '-f', '-h'));
354+
// [G] As discussed above, if hasParameterOption('-fh') is supported for
355+
// 'cli.php -f -h', then it seems it should also be supported
356+
// for 'cli.php -f -h'.
357+
// It seems reasonable to not support this.
358+
$this->assertTrue($input->hasParameterOption('-fh'), '->hasParameterOption() returns true if the given short option is in the raw input');
359+
}
360+
361+
public function testHasParameterOption()
362+
{
363+
$input = new ArgvInput(array('cli.php', '-f', 'foo'));
364+
$this->assertTrue($input->hasParameterOption('-f'), '->hasParameterOption() returns true if the given short option is in the raw input');
322365

323366
$input = new ArgvInput(array('cli.php', '--foo', 'foo'));
324367
$this->assertTrue($input->hasParameterOption('--foo'), '->hasParameterOption() returns true if the given short option is in the raw input');

0 commit comments

Comments
 (0)
0