[Unified Recorder] Add sample unified recorder tests to template#18817
Conversation
ghost
added
the
timovv
force-pushed
the
unified-recorder/18227-unified-recorder-in-template
branch
from
04daaec to
1eff9e6
Compare
timovv
requested review from
benbp,
deyaaeldeen,
mikeharder,
praveenkuttappan,
weshaggard and
witemple-msft
as code owners
|
Running into this issue when creating recordings in browser:
To reproduce, try running Tried to add the dotenv shim to the shared rollup config (see the PR changes) to no avail, but might be doing something wrong. @HarshaNalluru said you might be able to weigh in, @witemple-msft? |
timovv
force-pushed
the
unified-recorder/18227-unified-recorder-in-template
branch
from
1eff9e6 to
eee8f21
Compare
|
Still not quite ready, some tests aren't passing on the browser. Going to take another look tomorrow. |
timovv
force-pushed
the
unified-recorder/18227-unified-recorder-in-template
branch
from
19f2a94 to
ff90778
Compare
...configurationclient_functional_tests/recording_predetermined_setting_has_expected_value.json
Show resolved
Hide resolved
...dings/browsers/core_v1_tests_using_unified_recorder/recording_storagequeue_create_queue.json
Outdated
Show resolved
Hide resolved
jeremymeng
left a comment
jeremymeng
left a comment
There was a problem hiding this comment.
Should any new recordings be removed now that we don't have storage tests?
|
Yup, thought I'd removed them already! Think I got caught by #19045 again. I'll fix it up. |
witemple-msft
left a comment
witemple-msft
left a comment
There was a problem hiding this comment.
Removing my red review.
timovv
added
the
test-utils-recorder
| createPipelineFromOptions, | ||
| InternalPipelineOptions | ||
| createPipelineFromOptions | ||
| } from "@azure/core-http"; |
There was a problem hiding this comment.
Not related to this PR, I didn't realize we are still using core-http in the template. I think we should update to corev2. Filed #19447 to track
| "generate:client": "autorest --typescript ./swagger/README.md", | ||
| "integration-test:browser": "karma start --single-run", | ||
| "integration-test:node": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 5000000 --full-trace \"dist-esm/test/{,!(browser)/**/}/*.spec.js\"", | ||
| "integration-test:browser": "dev-tool run test:browser", |
There was a problem hiding this comment.
dev-tool run test:browser is a different script than the test:browser in line 48 right? Not a big deal but I was originally confused thinking this would end up in an infinite loop of "test:browser" -> "integration-test:browser" -> "test:browser".
I wonder if we can have a different name in dev-tool?
joheredi
left a comment
joheredi
left a comment
There was a problem hiding this comment.
It looks great! And changes to migrate an existing package seem straight forward which is great! Looking forward to try this out in a few packages once everything is merged in a single recorder package!
7 participants
Resolves #18227.
Needs updates as result of API changes:
TestProxyHttpClients for better clarity and Renaming #19446