Update WordPress Mocks to fix recent UI tests failures #17177
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
You can trigger an installable build for these changes by visiting CircleCI here. |
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
mokagio
changed the title
diegoreymendez
approved these changes
Sep 17, 2021
Contributor
diegoreymendez
left a comment
diegoreymendez
left a comment
There was a problem hiding this comment.
Once the pod is updated feel free to merge!
The reason I didn't notice the UI tests breaking is because I had enabled automerge on these PRs.
AliSoftware
added
the
[Status] Blocked
4 tasks
1 task
mokagio
removed
the
[Status] Blocked
Now that wordpress-mobile/WordPressMocks#35 has been merged and a new version shipped, we can point away from the WIP branch and back to a stable version.
mokagio
force-pushed
the
fix-ui-tests-crashing-on-missing-organization_id
branch
from
e8677de to
180122f
Compare
mokagio
deleted the
fix-ui-tests-crashing-on-missing-organization_id
branch
Contributor
Author
|
I updated the pod reference to the latest stable and run the UI tests on Buildkite and CircleCI: both passed 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The changes from the pair wordpress-mobile/WordPressKit-iOS#445 #17154 added a new property to a model deserialized from an already existing property in the JSON response (at least that's what I make of it by looking at the PRs and Git history see extra details here.
Because the property was already there in the live responses, everything worked well when tested manually, but the UI tests failed because the stubbed response they used did not include that key in the JSON. The tests crashed when trying to assign the
nilvalue stored in theorganizationIdproperty serialized from the stub response that didn't include the key.Related PRs
How did this happen?
#17154 was merged even though Buildkite reported the UI tests failing. This is understandable because we (the Owl team) haven't fully migrated to Buildkite for WordPress iOS yet and the setup has been having a few occasional issues.
Moreover, the UI tests did not run in CircleCI. That's understandable, too, because they are optional and need to be unlocked manually. There's no clear rule of thumb to know when to run them and when it's safe to skip them and not have to wait for them.
The good news is that, once we'll complete the migration to Buildkite, the CI performance will be such that UI tests will run on every commit, so this kind of lag in discovering out-of-date tests should disappear.