Skip to content

Updated getopt dependency to the latest version#23

Merged
Raibaz merged 1 commit intomasterfrom
upgrade-getopt-dependency
Feb 11, 2019
Merged

Updated getopt dependency to the latest version#23
Raibaz merged 1 commit intomasterfrom
upgrade-getopt-dependency

Conversation

@Raibaz
Copy link
Contributor

@Raibaz Raibaz commented Feb 9, 2019

This also fixes a test that fails with getopt v3.2.2 but passes with v3.0.

… was breaking on 3.2.2 but passing on 3.0

Change-Id: I8aca692518b75b6bbec20337cf738a3a0db2a50f
{
$this->expectOutputRegex('/Usage/');
$_SERVER['argv'] = [null, '--customerId', '--campaignId', 11111];
$_SERVER['argv'] = [null, '--campaignId', 11111, '--customerId'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be changed.
The test is aimed to test the case when customerId (which is required) is specified, but no value is given.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, but getopt changed its behavior between versions and now interprets '--campaignId' as the value for '--customerId', so to test the case when customerId is specified but no value is given this is correct.

Do you think we should add a specific test for the case when an example is invoked with something like --customerId --campaignId 111111 or should we just leave it as it is, having examples fail with request_error: Invalid customer ID '--campaignId'?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for confirming. Then, this already LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants