-
Notifications
You must be signed in to change notification settings - Fork 112
feat: add nav-data support to compare-api-responses script #385
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
Conversation
Vercel Previews Deployed
|
| } | ||
| } | ||
|
|
||
| async function fetchApiTypeVersionAndNav(options, product, versionMetadata) { |
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.
Since I split up version-metadata and nav-data options and nav-data uses a different response value than what is returned here, it made sense to me to pull this out and put it within each case
|
|
||
| const testsPassed = [] | ||
| const testsFailed = [] | ||
| const totalTests = [] |
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 added totalTests here since options.numOfTests may not be defined for every test run. This way the test results do not print 'Tests passed: 14 of undefined'
| totalTests.push(i + 1) | ||
| saveTestOutputIfSelected(outputString, newApiURL) | ||
| } | ||
| } else if (options.api === 'content-versions') { |
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.
'content-versions' got moved out of the for loops for versionmetadata and product because it does not use either thing in the loops. Since I removed the break statements for the loops, this would get run a bunch of times with the same output.
| break | ||
| if (difference.includes('Compared values have no visual difference.')) { | ||
| testsPassed.push(newApiURL) | ||
| console.log('✅ No visual difference found.\n') | ||
| } else { | ||
| testsFailed.push(newApiURL) | ||
| console.log(`${difference}\n`) | ||
| } | ||
|
|
||
| break |
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.
These break statements are removed because they are causing the tests to only run on the first version of the first product. Now, all products and versions get tests run on them
| `Tests passed: ${testsPassed.length} of ${options.numOfTests ?? totalTests.length} ${ | ||
| testsPassed.length === (options.numOfTests ?? totalTests.length) |
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.
Similar to comment above about totalTests. The options.numOfTests is not always defined so we need a fallback value
Description
This PR adds support for the nav-data endpoints to the compare-api-responses script. It also refactors a few parts of code to clean things up. I've added PR comments for more details on that refactoring.
Testing
First checkout this branch
Run the following commands to test the script:
One product:
npm run compare-api-responses -- -n https://web-unified-docs-hashicorp.vercel.app/ -o https://content.hashicorp.com -a nav-data -p terraform-cdkOne version for one product:
npm run compare-api-responses -- -n https://web-unified-docs-hashicorp.vercel.app/ -o https://content.hashicorp.com -a nav-data -p terraform-cdk -v v0.1 0.xAll products:
npm run compare-api-responses -- -n https://web-unified-docs-hashicorp.vercel.app/ -o https://content.hashicorp.com -a nav-data