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
Conversation
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') | |||
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.
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.
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, my plan was to investigate further to determine if we need this at all, but wanted to get CI fixed first.
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 see, nice catch by the way
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
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
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
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
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
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Update the API used to request a timestamp.