Skip to content
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

chore(tests): ensuring graphql responses don't change after launch #569

Merged
merged 33 commits into from
Apr 7, 2023

Conversation

bassrock
Copy link
Member

@bassrock bassrock commented Apr 6, 2023

Summary

Gain confidence in our test suite by shoring up it's reliability. There will be followup PRs to try and do this work iteratively.

See Implementation Details for more info.

References

  • Links to docs, tickets, designs if available

Implementation Details

  • Ensured that we never try and change what the GraphQL can respond with after the app was launched.
    • Audited all usages of the Mock GraphQL server
    • Made it so that fallbackResponses now contain all possible baseline responses
  • Ensured that where needed we were always awaiting the graphql server responses were fullfilled.
  • Switched to more specific request matching
  • Modified some test data to more accurately match what the server provides.
  • Updated the timeouts for Snowplow assertions
    • Also ensured that Debug builds emit analytics immediately instead of using a queue to speed up testing.
  • Fixed a bad access on persistent task

Test Steps

  • Run the full test suite. See no errors

PR Checklist:

  • Added Unit / UI tests
  • Self Review (review, clean up, documentation, run tests)
  • Basic Self QA

Screenshots

@pocket-ci
Copy link
Contributor

pocket-ci commented Apr 6, 2023

Messages
📖 No SwiftLint violations! 🎉
📖 Project coverage: 80.24%
📖 Edited 33 files
📖 Created 1 files

Analytics: Coverage: 85.42

File Coverage
PocketSnowplowTracker.swift 98.33%

Sync: Coverage: 88.28

File Coverage
PocketSource.swift 85.6%
FetchTags.swift 68.13%

Generated by 🚫 Danger Swift against cc2c82d

@bassrock bassrock changed the title chore(tests): delete tests chore(tests): ensuring graphql responses don't change after launch Apr 6, 2023
@bassrock bassrock marked this pull request as ready for review April 7, 2023 05:50
@bassrock bassrock requested a review from a team as a code owner April 7, 2023 05:50
@bassrock bassrock requested review from CMasterson and Gio2018 and removed request for a team April 7, 2023 05:50
Copy link
Contributor

@cyndichin cyndichin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! thanks

let addTagEvent = await snowplowMicro.getFirstEvent(with: "global-nav.addTags.addTag")
addTagEvent!.getUIContext()!.assertHas(type: "button")
addTagEvent!.getContentContext()!.assertHas(url: "http://localhost:8080/hello")
let events = await [snowplowMicro.getFirstEvent(with: "global-nav.addTags.removeInputTag"), snowplowMicro.getFirstEvent(with: "global-nav.addTags.addTag")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Collaborator

@Gio2018 Gio2018 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @bassrock ! Test were succeeding locally and remotely, even many of the "flakey" ones. Had one question

Comment on lines +86 to +88
space.backgroundContext.performAndWait {
persistentTask.currentCursor = pageInfo.endCursor
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed thanks for catching it!

}
app.launch()
app.tabBar.savesButton.wait().tap()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would we ever need to also wait for tabBar (or any other ancestor in any other test) existence?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think waiting for the savesButton inherently waits for the tab bar but in my experience here... adding more waits does not hurt anything at all.

@bassrock bassrock merged commit afc99c3 into develop Apr 7, 2023
@bassrock bassrock deleted the graphql-test-mutation branch April 7, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants