Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Civi\API\ExternalBatch - Improve test for variables_order/$_ENV #9969

Merged
merged 1 commit into from
Mar 13, 2017

Conversation

totten
Copy link
Member

@totten totten commented Mar 13, 2017

The recent revision #9595 updated ExternalBatch to produce a more
informative error message. However, in the test environment used by
flexmailer, there was exactly 1 value in $_ENV (even if
variables_order was misconfigured).

This should make the test a bit harder to trip-up by affirmatively checking
for the most common environment variable.

Review note: In the known universe, the only direct references to
ExternalBatch are in CiviUnitTestCase, so this shouldn't impact any
runtime logic.

The recent revision civicrm#9595 updated `ExternalBatch` to produce a more
informative error message.  However, in the test environment used by
`flexmailer`, there was exactly 1 value in `$_ENV` (even if
`variables_order` was misconfigured).

This should make the test a bit harder to trip-up by affirmatively checking
for the most common environment variable.

Review note: In the known universe, the only direct references to
`ExternalBatch` are in `CiviUnitTestCase`, so this shouldn't impact any
runtime logic.
@eileenmcnaughton
Copy link
Contributor

I think Jenkins is the appropriate reviewer for this patch as it affects tests only, and is an 'author-update'

@eileenmcnaughton eileenmcnaughton merged commit e504119 into civicrm:master Mar 13, 2017
monishdeb pushed a commit to monishdeb/civicrm-core that referenced this pull request May 2, 2017
Civi\API\ExternalBatch - Improve test for variables_order/$_ENV
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants