-
Notifications
You must be signed in to change notification settings - Fork 120
[HACK Week] Attach connectivity test log to Zendesk issue #16449
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
Conversation
326c7a8 to
6db510c
Compare
|
|
itsmeichigo
left a comment
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.
Thanks for working on this! I tested both the happy path and an edge case - I have a question regarding the edge case below 👇
| latestTestResult.append(ConnectivityTestResult(testCase: testCase, | ||
| result: testResult, | ||
| timeTaken: timeTaken)) |
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 tested an edge case:
- Run the tests and stub one of the request to fail with decoding error.
- Retry the same test but let it succeed.
- Result: I see both failure and success in the log:
## 1. Internet Connection
Took: 0ms
Result: Success
## 2. Connecting to WordPress.com Servers
Took: 721ms
Result: Success
## 3. Connecting to your site
Took: 1567ms
Result: Success
## 4. Fetching your site orders
Took: 1940ms
Result: Success
## 5. Fetching products in your store
Took: 12208ms
Result: Operation: Fetching products in your store
Error Type: Decoding Error
Issue: Required Key Not Found
Missing Key: id
Coding Path: data → Index 0
Description: No value associated with key CodingKeys(stringValue: "id", intValue: nil) ("id").
## 6. Fetching products in your store
Took: 1756ms
Result: Success
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.
Thanks for pointing this out. I've intentionally decided to keep every "test run" and not overwrite the original run when retrying a test. This way, we can see the full picture and not just the last "snapshot". And since we are attaching the report, the report size is not a constraint.
Would you prefer overwriting with the retry?
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 I have mixed feelings about this but it might not be straightforward to replace the results. Sometimes intermittent failures are also a sign of something broken on a site. Let's keep this for now then.
| private var caseName: String { | ||
| switch testCase { | ||
| case .internetConnection: "Internet Connection" | ||
| case .wpComServers: "Connecting to WordPress.com Servers" | ||
| case .site: "Connecting to your site" | ||
| case .siteOrders: "Fetching your site orders" | ||
| case .loadingProducts: "Fetching products in your store" | ||
| } |
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.
We can simply use testCase.title for this.
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.
From a technical point of view, yes, we could. But testCase.title is localized, so the HEs would have to translate it to see which step it was. We are making sure the report will have English titles by using caseName here.
itsmeichigo
left a comment
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.
Approving as the comments above are not blocking.

This is a follow-up on HACK Week: Add details for decoding errors in troubleshooting tool
Description
This PR attaches the troubleshooting tool's output as an attachment to the Zendesk issue if you navigate to the Contact Support page.
Test Steps
7 . Fill out required fields.
Screenshots
RELEASE-NOTES.txtif necessary.