-
Notifications
You must be signed in to change notification settings - Fork 646
chore: convert bdd tests to e2e tests #7539
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
base: main
Are you sure you want to change the base?
Conversation
79c8ad9 to
e15aef3
Compare
| "extract:docs": "api-extractor run --local", | ||
| "generate:client": "node ../../scripts/generate-clients/single-service --solo acm" | ||
| "generate:client": "node ../../scripts/generate-clients/single-service --solo acm", | ||
| "test:e2e": "yarn g:vitest run -c vitest.config.e2e.mts" |
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.
include a test:e2e:watch script counterpart for all test scripts
|
|
||
| expect(result).toBeDefined(); | ||
| expect(result.CertificateSummaryList).toBeDefined(); | ||
| expect(Array.isArray(result.CertificateSummaryList)).toBe(true); |
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.
no need for a triple chain of assertions, since the third one covers all 3
|
|
||
| expect(result).toBeDefined(); | ||
| expect(result.items).toBeDefined(); | ||
| expect(Array.isArray(result.items)).toBe(true); |
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.
only need one assertion
| }); | ||
|
|
||
| describe("Describing stacks", () => { | ||
| it("should return stacks list when describing stacks is called", async () => { |
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.
DescribeStacks is the operation name, not describing stacks
| for await (const page of paginator) { | ||
| pageCount++; | ||
| lastPage = page; | ||
| if (pageCount >= 3) break; // Get at least three pages |
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.
this test doesn't match the original test
"the last page must not contain a marker" i'm guessing this refers to the pagination token
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.
Yes you are right.
Issue
Issue number, if available, prefixed with "#"
Description
What does this implement/fix? Explain your changes.
Testing
How was this change tested?
Additional context
Add any other context about the PR here.
Checklist
*.integ.spec.ts).@publictag and enable doc generation on the package?By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.