test(workflow): add pptr link checker #3526
Conversation
This reverts commit 628100a.
|
| @@ -1,3 +1,3 @@ | |||
| export const PROMISE_POOL_CONCURRENCY = 10; | |||
| export const PROMISE_POOL_CONCURRENCY = 5; | |||
There was a problem hiding this comment.
Fix the memory leak.
Since the Github Action Step didn't change after reducing the PROMISE_POOL_CONCURRENCY to 5, we don't have to test numbers bigger than 5 to reduce the testing time.
| statusCode, | ||
| }) | ||
| )?.statusCode || statusCode; | ||
| )?.statusCode ?? statusCode; |
There was a problem hiding this comment.
This is the major change of this new PR 👿
There was a problem hiding this comment.
Can you please explain what this change does?
There was a problem hiding this comment.
Yes. Good question.
If a link is good, we return a statusCode: 0 (code). The problems was, the redirected link got a 0, and it was 308 before redirected, so 0 || 308 and we got 308.
We should have been able to filter out the 308 errors when reporting result (code), but since the SITE_URL Github Env Variable was not set to be localhost:3000 (Github Env Variable generates the sitemap, meaning the link checker was not testing against 'localhost:3000'), it was not filtered out.
That's why we got those internal 308 errors which failed the build.
Since we've changed the Github Env Variable, the problem should be resolved.
So, it's actually not necessary to change || to ?? for internal 308 redirection.
However, since we have another logic to test 301 from docs.amplify.aws (code), this change is still useful because we don't have a filter when reporting result.
|
|
||
| </Alert> | ||
|
|
||
| For your [configured social providers](https://docs.amplify.aws/lib/auth/social_signin_web_ui/q/platform/flutter/), you can configure Apple, Amazon, Facebook, or Google: |
There was a problem hiding this comment.
3 participants
Original PR #3516 had to be reverted #3523 because it report statasCode 308 from internal links as errors (build).
It was because if the returned status is
0, it would keep using308.Why didn't the issue happen in PoC?
Because the PoC fork filtered the
308byisInternalRedirection, but this main repo had a differentSITE_URL, which might be the dev site, so it was likely testing the links on the dev site.Description of changes
||to??to catch0Issue #, if available
Description of how you validated changes
Tested in PoC draft PR #3524:
promisePoolConcurrencyto5https://github.com/aws-amplify/amplify-ui/actions/runs/4369009192/jobs/7642422394||to??https://github.com/aws-amplify/amplify-ui/actions/runs/4369275515/jobs/7642995310 and caught a 404 linkSITE_URLand test passed https://github.com/aws-amplify/amplify-ui/actions/runs/4370516636/jobs/7645573316Checklist
yarn testpassessideEffectsfield updatedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.