Live tests for managed identity, username/password, browser credentials#7622
Live tests for managed identity, username/password, browser credentials#7622chlowell merged 11 commits intoAzure:masterfrom
Conversation
chlowell
added
Client
|
Can one of the admins verify this patch? |
chlowell
force-pushed
the
identity-live
branch
from
23fc6f5 to
7188cef
Compare
|
/check-enforcer evaluate |
chlowell
force-pushed
the
identity-live
branch
from
7188cef to
aef4e2f
Compare
chlowell
changed the title
chlowell
force-pushed
the
identity-live
branch
2 times, most recently
from
1c2e470 to
1a65707
Compare
KieranBrantnerMagee
left a comment
KieranBrantnerMagee
left a comment
There was a problem hiding this comment.
LGTM, two small comments that might improve clarity.
There was a problem hiding this comment.
"paranoia-for-future-me-being-dumb" comment incoming: Your commit message gives good detail on usage of these flags, but as an outsider coming in, I'm not sure if it's at all discoverable. Is there a readme/other source of knowledge for identity tests? (in a perfect world I'd want to plumb this into proper args and give it details in -h, but I would guess that's hard because identity is just one possible configuration within our generic pytest setup?)
There was a problem hiding this comment.
This particular line was an ugly hack to determine whether pytest was launched with '-m manual', so we can use the 'manual' marker to opt in to manual tests. To your broader point, all knowledge outside the code is tribal. I have a tracking issue for documenting the test matrix (#8368) but there's higher priority test work likely to obsolete any document written today, e.g. setting up recording infrastructure (#5980).
There was a problem hiding this comment.
My first thought reading this was that prints is an unused parameter, but realized it was a fixture you wrote to skip this test. That pattern wasn't super intuitive to me; if it's consistent with broader use or pytest practices feel free to ignore this, but a "check and skip" seems like more of a decorator than a fixture?
There was a problem hiding this comment.
Thanks, that's a good suggestion, I replaced the fixture with a marker.
chlowell
force-pushed
the
identity-live
branch
from
1a65707 to
71b3ad1
Compare
KieranBrantnerMagee
left a comment
KieranBrantnerMagee
left a comment
There was a problem hiding this comment.
LGTM, one comment that may just be paranoia.
| # Ignore async tests on unsupported platforms | ||
| if sys.version_info < (3, 5): | ||
| collect_ignore_glob.append("*_async.py") | ||
| collect_ignore_glob = ["*_async.py"] |
There was a problem hiding this comment.
should we have a collect_ignore_glob = [] above given that we nixed the imds initialization?
There was a problem hiding this comment.
No, pytest doesn't require a value for collect_ignore_glob.
* Updated minor version for released packages in hotfix branch * Refactor ClientCertificateCredential construction (#8315) * Update MSAL dependency (#8359) * Fix InteractiveBrowserCredential caching (#8352) * Disable depends test (#8440) * [EventHubs] Final README update (#8430) * [EventHubs] Final docs update (#8428) * Readme final update * Update links * update async sample in readme * Simply samples in readme and make samples can run (#8432) * Tiny readme fix * Update checkpointstore README * Update checkpointstore README * Fix a README error * App Configuration 2019-10-01 (#8394) * update sdk\keyvault\azure-keyvault-secrets\README.md (#8396) * bump warden version (#8460) * Add passing kwargs and HttpLoggingPolicy plugged (#8053) * Live tests for managed identity, username/password, browser credentials (#7622) * policy v2019_09_01 (#8397) * Fix media streaming bug (#8385) * initial commit to fix media streaming bug - Removed media streaming related checks in synchronized_request that was causing issues with DBAs with "media" in the name - Refactored pipeline_client.run into a _PipelineRun method for testing purposes * bumped version * removed references to 'Media' * bumped version in readme * removed unused import for pylint * undid test config changes * updated HISTORY with breaking changes * modified changelog * Work around sphinx markdown heading link bug (#8255) * move smoke tests update smoke-test.yml move smoketests to common folder * consistent smoke test requirements * remove smoke test invocations that don't exist * use original interface * remove phantom smoke tests * New CI to check autorest when changes are made in azure-core (#8509) * ADLS Gen2 API Implementation (#8473) * ADLS Gen2 API Implementation (#8473) * network parameter change (#8529) * [AutoPR] appconfiguration/resource-manager (#8528) * New azure-mgmt-web + fixed mixin script (#8334) * list skus test * adding api_version in mixin instance * regenerated new package * regeneated tests * history and version * [AutoPR] cognitiveservices/data-plane/LUIS/Authoring (#7816) * [AutoPR cognitiveservices/data-plane/LUIS/Authoring] Add v3.0 swagger (#7815) * Generated from 7a554c128d6eba201c576885c8c2ea78f5267481 Add v3.0 swagger * Generated from d3a4f2d1674a63bc9a4a57015f956833c7d40f7b Update LUIS-Authoring.json fix semantic bugs * regenerated luis * updated history and version
3 participants
These require specific conditions to run:
$TEST_IMDS(tester must opt-in because one can discover IMDS support only by trying to use it)$MANAGED_IDENTITY_ID$AZURE_USERNAME,$AZURE_PASSWORD, and$USER_TENANTpytest ... -m manualpytest ... -sto make that visibleWhich is all to say they won't run in CI today. The primary value of this change is that is simplifies running them manually.