[Text Analytics] Adding TEST_AUTH_MODE env var

[deyaaeldeen]

Fixes #11043.

I am proposing something like this for other packages as well to parameterize the test suite over the method of authentication. What do you guys think?

[willmtemple]

Hmm. So I recognize the appeal of having this option to be able to choose whether or not the tests run using AAD RBAC vs. static keys, e.g.:

• running all the tests against private endpoints where we have one type of cred but not the other
• revealing if any tests have failures/different behavior using only one type of authentication (which is rare but does occasionally occur and we are usually the ones that catch it)

Some thoughts on this approach though:

One thing that comes to mind is that it looks like the variable is not so much for selecting which type of credential is used, but rather for optionally forcing API Key authentication in the existing AAD tests. In other words, if I pass TEST_AUTH_MODE="AAD" in the environment, the behavior is no different from if it was not specified at all, and I will still get API Key credentials in the tests that are still labeled API Key. If that's the case, why not do a variable that carries some of that meaning. FORCE_TEST_API_KEY_AUTH=true or maybe FORCE_TEST_AUTH_MODE="AAD" and that will run all the tests with AAD or API Key.

Alternatively, we could have just one set of tests that we run once with API Key and then again with AAD, and we could have a variable that inhibits one of the two.

The implementation in this PR seems like it's kind of in the middle somewhere. We also need to make sure that the sample.env documentation of what the variable controls is precise.

[deyaaeldeen]

@willmtemple Thanks for the great feedback!

Alternatively, we could have just one set of tests that we run once with API Key and then again with AAD, and we could have a variable that inhibits one of the two.

My thought process was to enable tests with a fixed authentication method to live side-by-side with others who are parameterized. However now that I think about it, all tests should work with any authentication method, at least for text analytics.

[willmtemple]

@deyaaeldeen I added a utility function called matrix that I have used to unify AAD/API Key Form Recognizer training test suites. You can take a look at sdk/formrecognizer/ai-form-recognizer/test/utils/matrix.ts for the source and at sdk/formrecognizer/ai-form-recognizer/test/public/formtrainingclient.spec.ts for an example of how I use it. It doesn't support what you're talking about here with being able to opt out of one or the other, but it might be a good jumping-off point for getting this functionality working elegantly.

[HarshaNalluru]

Interesting!
For other libraries, take service-bus for example, we have all the tests running only with the connection string and a single test specific for AAD to make sure the auth works. Same with storage libraries I believe.

With this new mode,

• How would you want to manage the recordings? (Would be different for "APIKey" vs "AAD" but the test title remains the same)
• How often do you want to run these tests with different modes? (Live tests would pick the auth mode at random?)

[willmtemple]

@HarshaNalluru the issue is that sometimes a preview/canary deployment fails only some endpoints or methods with AAD. In FR I've just decided to run all tests on both AAD and using API Keys, so I'll see if anything fails and where. Sometimes I want to disable AAD tests for one reason or another, and so it would be nice to have "AAD + API Key" be the default, but optionally be able to just run with API Key".

Another question is about services that support more than just these two authentication methods, for example anonymous auth in storage.

[deyaaeldeen]

@HarshaNalluru

• we can generate tests dynamically if we need to. However, I am not able to see why we would ever need to record tests in both modes.
• this can be a discussion for later with the team. For text analytics, I often need to run the tests in both modes closer to our releases.

@willmtemple

Another question is about services that support more than just these two authentication methods, for example anonymous auth in storage.

I would imagine this approach is simple enough to accommodate those scenarios as well. Each package needs to provide an adapter that creates the credential the user requested.

[HarshaNalluru]

However, I am not able to see why we would ever need to record tests in both modes.

@deyaaeldeen Do the recordings stay the same irrespective of the auth mode?

[sadasant]

I'm a bit confused on why this is needed!

Generally, if there's an issue with the authentication methods, we should fix and write tests in Identity.

Is this because of a customer facing issue? Or is this because the identity tests are insufficient?

[willmtemple]

Generally, if there's an issue with the authentication methods, we should fix and write tests in Identity.

@sadasant This is not a bug in Identity. If there were an issue with the authorization flows, then we would fix that in identity and add tests there. This is a developer quality-of-life thing for us. Sometimes early service deployments (preview/ppe/canary, etc.) may have problems authenticating our tokens, so this is a convenience feature for us to be able to inhibit the AAD tests without having to go in and insert a lot of .only into our tests.

[sadasant]

@deyaaeldeen if this is about testing a credential that only lives in the Text Analytics client, I believe we should test the credential as we're testing the other credentials in identity! If that makes sense.

[sadasant]

After talking about this PR, I understand what's going on and this looks good to me!

[sadasant]

I will get in contact with the Identity Crew about the idea of moving AzureKeyCredential to Identity, and I'll get in contact with Engineering about potentially running all client tests with all the available credentials.

[deyaaeldeen]

Now that textanalytics has tests that only work with API Key, we need to rethink our approach to this. I will close it for now.