-
Notifications
You must be signed in to change notification settings - Fork 7
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(tests): battle hardened tests against multiple device sizes #576
Conversation
@@ -21,7 +21,8 @@ class TagsFilterViewModel: ObservableObject { | |||
} | |||
|
|||
func getAllTags() -> [TagType] { | |||
arrangeTags(with: fetchedTags?.compactMap({ $0.name }) ?? []) | |||
// Finding unique elements using set | |||
arrangeTags(with: Array(Set(fetchedTags?.compactMap({ $0.name }) ?? []))) |
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 had to do this, because at times the FilterTagsView would contain duplicate tags. Should be fixed in #568
allTags.append(contentsOf: fetchedTags) | ||
} | ||
return allTags.compactMap { TagType.tag($0) } | ||
return tags.sorted().compactMap { TagType.tag($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.
removing recent tags instead for #568
@@ -212,8 +212,8 @@ workflows: | |||
## It uses the values in MICRO_IGLU_REGISTRY_URL and MICRO_IGLU_API_KEY | |||
echo "Run snowplow" | |||
brew install wget openjdk | |||
wget https://github.com/snowplow-incubator/snowplow-micro/releases/download/micro-1.5.0/snowplow-micro-1.5.0.jar | |||
java -jar snowplow-micro-1.5.0.jar & | |||
wget https://github.com/snowplow-incubator/snowplow-micro/releases/download/micro-1.6.0/snowplow-micro-1.6.0.jar |
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.
updating snowplow
PocketKit: Coverage: 79.75
Sync: Coverage: 88.23
Generated by 🚫 Danger Swift against be61cce |
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.
looks cool! excited for these changes
Summary
@timc-mozilla discovered that our test was not battle hardended against small device sizes. This makes use of a scrollTo function to ensure that we scroll to a specific element instead of unless scrolling or assuming something is on screen.
This will also fixes flakyness I discovered around tags where tags were always appearing in a different order. This should be fully fixed in #568 and any work here is a stop gap for tests and sanity.
Implementation Details
There are a few helper libraries we could import that would help with our tests, but I chose to use a very basic scrollTo function instead since the one I found had not been updated in a while.
Test Steps
PR Checklist:
Screenshots