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

Add easy record steps to builds #4389

Closed
sophiajt opened this issue Jul 23, 2019 · 5 comments
Closed

Add easy record steps to builds #4389

sophiajt opened this issue Jul 23, 2019 · 5 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system.

Comments

@sophiajt
Copy link
Contributor

Currently, in order to record test results for playback, we have to carry along a lot of environmental settings. This makes it a bit cumbersome to re-record tests once changes have occurred.

It'd be good to be able to have the necessary settings picked up from the environment (and then stripped away as necessary) via an npm build step (eg: record-browser or record-node)

@loarabia loarabia added Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. and removed triage labels Jul 26, 2019
@sadasant
Copy link
Contributor

Jonathan, the only thing needed is the TES_MODE environment variable. What are we missing?

@sophiajt
Copy link
Contributor Author

Last I tried it, you had to pass in all the environment variables the test used (including the CLIENT_ID, etc that EnviromentCredentials used). This is a bit lengthy and error-prone, so why not just have the recorder (if possible) know what environment variables it needs so that it can store them for the playback

sadasant added a commit to sadasant/azure-sdk-for-js that referenced this issue Jul 30, 2019
With this change, we can have a `.env` file in `sdk/keyvault` (the parent
folder, as well as with other of our projects using dotenv) with the
following properties:

```
AZURE_CLIENT_ID=XXXXXXXX
AZURE_CLIENT_SECRET=XXXXXXXX
AZURE_TENANT_ID=XXXXXXXX
KEYVAULT_NAME=XXXXXXXX
TEST_MODE=["record" or "playback", without quotes]
```

And then run `rushx integration-test:node` to record (or playback) all
the tests with the Node environment, and also `rushx integration-test:browser`
to run the tests with the browser environment.

Would this help with Azure#4389 ?

Please review 💙
@sadasant
Copy link
Contributor

sadasant commented Jul 30, 2019

The recorder already replaces all the environment variables with stub values! Only TEST_MODE=playback is needed to run the playback tests.

@sophiajt
Copy link
Contributor Author

Yes, though this was around what you pass to the recorder to do a recording. Using the .env file might be a better fix, which is sounds like the direction we're going.

sadasant added a commit that referenced this issue Jul 30, 2019
With this change, we can have a `.env` file in `sdk/keyvault` (the parent
folder, as well as with other of our projects using dotenv) with the
following properties:

```
AZURE_CLIENT_ID=XXXXXXXX
AZURE_CLIENT_SECRET=XXXXXXXX
AZURE_TENANT_ID=XXXXXXXX
KEYVAULT_NAME=XXXXXXXX
TEST_MODE=["record" or "playback", without quotes]
```

And then run `rushx integration-test:node` to record (or playback) all
the tests with the Node environment, and also `rushx integration-test:browser`
to run the tests with the browser environment.

Would this help with #4389 ?

Please review 💙
@sadasant
Copy link
Contributor

I'm closing this for now! Please open again if I'm missing something ❤️

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system.
Projects
None yet
Development

No branches or pull requests

3 participants