change iap assert order #4515
change iap assert order #4515
Conversation
iap/iap_test.py
Outdated
|
|
||
| assert 'JWT valdiation error' not in out |
There was a problem hiding this comment.
I don't think that stdout contains the error message.
python-docs-samples/iap/validate_jwt.py
Line 46 in dfe31fb
It's returning a tuple and jwt_validation_result[2] has the error message.
There was a problem hiding this comment.
Those two lines are wrong. It looks like they were put in by the merge with master. I'll take them back out.
|
So now this PR doesn't have any changes? I'm not sure if it will solve the original issue. |
|
I can't tell from the original issue whether this test really is flaky, or if it was a one-time thing. I can't find other failures of this test. There may be nothing to solve. |
|
Lol alright - I trust you @engelke - I was mostly guessing |
|
@engelke if flaky, is it obvious what the underlying assertion is that we could potentially use with the |
|
There are only two ways I can see for this to fail. First, if the app at gcp-devrel-iap-reflect.appspot.com doesn't return the correct response, or if the URL for the validation certs doesn't return the right certs. I guess the metadata service could also fail to return a valid token when asked. I'd guess the reflection appspot app might be a bit more likely to have a problem, perhaps due to a very slow startup once in a while. It's hard to see how to fix it without just rerunning the whole test. |
|
My guess is that the code throws an exception. More problematically, the test code is throwing away the error, by asserting the first element of the tuple. My suggestion is that, first we need to log the error. |
|
I don't think we need to "Update branch", feel free to merge. Also feel free to "Update branch" then merge :) |
Description
Fixes #4467
Note: It's a good idea to open an issue first for discussion.
Checklist
nox -s py-3.6(see Test Environment Setup)nox -s lint(see Test Environment Setup)