Skip to content
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

fix(bazel): fix integration test for bazel building #38629

Closed
wants to merge 1 commit into from

Conversation

josephperrott
Copy link
Member

@josephperrott josephperrott commented Aug 28, 2020

Update the API used to request a timestamp.

Copy link
Contributor

@atscott atscott left a comment

LGTM since the test now passes but can you update the commit message to be a bit more descriptive about why this change was made? It currently just says "what" the change is but not "why" it was necessary.

Update the API used to request a timestamp.  The previous API we relied on for this
test application, worldclockapi.com no longer serves times and simply 403s on all
requests.  This caused our test to timeout as the HTTP request did not handle a failure
case.  By moving to a new api, the HTTP request responds as expected and timeouts
are corrected as there is not longer a pending microtask in the queue.
@@ -15,6 +15,6 @@ export class AppComponent {
constructor(private http: HttpClient) {}

time$: Observable<string> =
this.http.get('http://worldclockapi.com/api/json/pst/now')
.pipe(map((result: any) => result.currentDateTime), startWith(['...']));
this.http.get('http://worldtimeapi.org/api/timezone/America/Los_Angeles.json')
Copy link
Member

@devversion devversion Aug 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity: Should we just change this away from an external server? I don't see why we'd need to rely on an external server here? Especially as that can impact our stability.

Copy link
Member Author

@josephperrott josephperrott Aug 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my plan was to investigate further to determine if we need this at all, but wanted to get CI fixed first.

Copy link
Member

@devversion devversion Aug 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, nice catch by the way 😄!

josephperrott added a commit that referenced this issue Aug 28, 2020
Update the API used to request a timestamp.  The previous API we relied on for this
test application, worldclockapi.com no longer serves times and simply 403s on all
requests.  This caused our test to timeout as the HTTP request did not handle a failure
case.  By moving to a new api, the HTTP request responds as expected and timeouts
are corrected as there is not longer a pending microtask in the queue.

PR Close #38629
josephperrott added a commit that referenced this issue Aug 28, 2020
Update the API used to request a timestamp.  The previous API we relied on for this
test application, worldclockapi.com no longer serves times and simply 403s on all
requests.  This caused our test to timeout as the HTTP request did not handle a failure
case.  By moving to a new api, the HTTP request responds as expected and timeouts
are corrected as there is not longer a pending microtask in the queue.

PR Close #38629
@josephperrott josephperrott deleted the bazel-test-fix branch Aug 28, 2020
subratpalhar92 added a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this issue Aug 29, 2020
Update the API used to request a timestamp.  The previous API we relied on for this
test application, worldclockapi.com no longer serves times and simply 403s on all
requests.  This caused our test to timeout as the HTTP request did not handle a failure
case.  By moving to a new api, the HTTP request responds as expected and timeouts
are corrected as there is not longer a pending microtask in the queue.

PR Close angular#38629
subratpalhar92 added a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this issue Aug 31, 2020
subratpalhar92 added a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this issue Aug 31, 2020
Update the API used to request a timestamp.  The previous API we relied on for this
test application, worldclockapi.com no longer serves times and simply 403s on all
requests.  This caused our test to timeout as the HTTP request did not handle a failure
case.  By moving to a new api, the HTTP request responds as expected and timeouts
are corrected as there is not longer a pending microtask in the queue.

PR Close angular#38629
profanis added a commit to profanis/angular that referenced this issue Sep 5, 2020
Update the API used to request a timestamp.  The previous API we relied on for this
test application, worldclockapi.com no longer serves times and simply 403s on all
requests.  This caused our test to timeout as the HTTP request did not handle a failure
case.  By moving to a new api, the HTTP request responds as expected and timeouts
are corrected as there is not longer a pending microtask in the queue.

PR Close angular#38629
@angular-automatic-lock-bot
Copy link

@angular-automatic-lock-bot angular-automatic-lock-bot bot commented Sep 28, 2020

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants