Skip to content

Conversation

@timovv
Copy link
Member

@timovv timovv commented Dec 10, 2021

@timovv timovv requested a review from HarshaNalluru December 10, 2021 21:34
@timovv timovv added the test-utils-recorder Label for the issues related to the common recorder label Dec 10, 2021
@timovv timovv marked this pull request as ready for review December 13, 2021 19:39
@timovv timovv requested a review from sadasant as a code owner December 13, 2021 19:39
@timovv
Copy link
Member Author

timovv commented Jan 7, 2022

We should add a troubleshooting section that describes how to run the recorder manually and get logs.

Comment on lines +34 to +35
- `node-ts-input` runs the tests using `ts-node`, without code coverage.
- `node-js-input` runs the tests using the built JavaScript output, and generates coverage reporting using `nyc`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `node-ts-input` runs the tests using `ts-node`, without code coverage.
- `node-js-input` runs the tests using the built JavaScript output, and generates coverage reporting using `nyc`.
- `node-ts-input` runs the tests using `ts-node`(takes TS files as input), without code coverage.
- `node-js-input` runs the tests using the test (JS) bundle built with rollup, and generates coverage reporting using `nyc`.

});
```

To enable the recorder, you should then initialize your SDK client as normal and use the recorder's `configureClient` method. This method will attach the necessary policies to the client for recording to be enabled.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also mention that the client should be exposing the "pipeline" object to get the policy added properly.

Copy link
Member Author

@timovv timovv Jan 13, 2022

Choose a reason for hiding this comment

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

It would be a compile error if pipeline isn't exposed, but I'll add a note


Once you have made the necessary code changes, it is time to re-record your tests using the Unified Recorder to complete the migration. To do this, first **delete** the directory containing the old recordings (the `recordings` folder). Then, run your tests with the `TEST_MODE` environment variable to `record`.

If everything succeeds, the new recordings will be made available in the `recordings` directory. Inspect them to make sure everything looks OK (no secrets present, etc.), and then run the tests in playback mode to ensure everything is passing. If you're running into issues, check out the [Troubleshooting section](#troubleshooting).
Copy link
Contributor

Choose a reason for hiding this comment

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

troubleshooting is empty?

Copy link
Member Author

Choose a reason for hiding this comment

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

uh oh...

why do I remember writing it if it's not there...

docker run -v <your azure-sdk-for-js repository root>:/srv/testproxy -p 5001:5001 -p 5000:5000 -e Logging__LogLevel__Microsoft=Debug azsdkengsys.azurecr.io/engsys/testproxy-lin:latest
```

Once you've done this, you can run your tests in a separate terminal. `dev-tool` will detect that a test proxy container is already running and will point requests to the Docker container you started.
Copy link
Contributor

Choose a reason for hiding this comment

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

perfect 🎉

@timovv timovv merged commit e00dc6d into Azure:main Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test-utils-recorder Label for the issues related to the common recorder

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants