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
[Search] Refactor tests #29595
[Search] Refactor tests #29595
Conversation
b5007b7
to
b27eb9c
Compare
343602a
to
cc2ced3
Compare
API change check API changes are not detected in this pull request. |
cc2ced3
to
e4cb613
Compare
564fd36
to
9ba86c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few nits - I trust your judgement on whether you want to take them in or not - otherwise
sdk/search/search-documents/test/public/node/searchClient.spec.ts
Outdated
Show resolved
Hide resolved
} from "../../../src"; | ||
import { delay } from "../../../src/serviceUtils"; | ||
import { COMPRESSION_DISABLED } from "../../compressionDisabled"; | ||
import { Hotel } from "./interfaces"; | ||
|
||
export const WAIT_TIME = isPlaybackMode() ? 0 : 4000; | ||
|
||
function uniqueNameGenerator() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a uuid
from @azure/core-util work here instead? Meaning deleting this helper and just using uuids for unique names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like for these names to be human-readable and deterministic. Maybe we can leverage a hash function instead?
9ba86c7
to
1ef237d
Compare
1ef237d
to
4d9522a
Compare
assert.equal(correctServiceVersion, client.apiVersion); | ||
}); | ||
|
||
it("prioritizes `serviceVersion` over `apiVersion", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a common miss when deprecating a field for another field (wha thappens if I pass both) - glad you're thinking and testing this!
4d9522a
to
bd464de
Compare
/check-enforcer override |
Packages impacted by this PR
@azure/search-documents
Describe the problem that is addressed by this PR
A small refactor of tests to support 2024-05 test cases
Designed to be reviewed on a per-commit basis.
Depends on #29600
Command used to generate this PR:**(Applicable only to SDK release request PRs)
Checklists