-
Notifications
You must be signed in to change notification settings - Fork 515
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
[Tests] Try different urls before failing the test. #6237
[Tests] Try different urls before failing the test. #6237
Conversation
We do have a test that fails when there are issues accessing the remote resource. If an error is raised, rather than setting the tests as an error, set them to inconclusive with the message of the error. Related issue: xamarin/maccore#1725
Assert.Inconclusive ($"Could not access url error is '{error.Description}'"); | ||
} else { | ||
Assert.That ((x != null) && (x.Length > 0)); | ||
} |
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.
the test was to detect an error...
with that change I don't think you can trigger the 2nd assert (unless an empty file was present)
so basically the test can't fail (so it's not worth much)
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.
@spouliot we can check the domain of the error and decide what to do?
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.
Well.. thx Cocoa, we would get the not so useful "NSCocoaErrorDomain Code=256", which means an unknown error happened.
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.
yeah, a reachability (to host) error would be fine to ignore, but other errors would not be
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 have used several company urls, lets hope trying several will work, else we can do an increasing timeout retry.
Build failure |
for reference
https://jenkins.mono-project.com/job/xamarin-macios-pr-builder/10214/Test_20Report/ |
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.
Two ideas to improve this test:
- Ignore only when doing CI (like 7d67e05), this way we'll eventually see failures when running into them locally.
- Have a list of URLs to try, and pass the test if any of them work.
it's unclear why it fails so it's possible the complete list will fail (even more since it's all MS hosts) but if it works then it's much better than the current test :) |
Build failure Test results4 tests failed, 90 tests passed.Failed tests
|
Issues are related to the msbuild problems we have, all other tests, specially the NSDataTests passed. Known issues are: |
@monojenkins backport to d16-2 |
@monojenkins backport to d16-1 |
@monojenkins backport to xcode11 |
We do have a test that fails when there are issues accessing the remote
resource. If an error is raised, rather than setting the tests as an
error, set them to inconclusive with the message of the error.
Related issue: https://github.com/xamarin/maccore/issues/1725