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

[BUDI-8579] Create more realistic tests for Google Sheets #14535

Merged
merged 15 commits into from
Sep 10, 2024

Conversation

samwho
Copy link
Collaborator

@samwho samwho commented Sep 6, 2024

Description

In this PR I've completely overhauled how we test our Google Sheets integration. I've created a new utility class called GoogleSheetsMock whose job it is to intercept Google Sheets API calls via nock and maintain the state of a mocked Spreadsheet within itself. This makes it a crude but useful simple reimplementation of the Google Sheets API that we can write tests against.

I'm about 90% sure in the process of doing this, and in #14542, we've fixed the linked bug.

I want to do at least one follow-up PR adding a few more tests, mainly around deletes.

Addresses

Launchcontrol

Fixes several bugs around creating and updating rows in Google Sheets.

Copy link

linear bot commented Sep 6, 2024

@github-actions github-actions bot added firestorm Data/Infra/Revenue Team size/l labels Sep 6, 2024
@github-actions github-actions bot added size/xl and removed size/l labels Sep 9, 2024
@samwho samwho changed the title Budi 8579 issue with google sheets [BUDI-8579] Create more realistic tests for Google Sheets Sep 10, 2024
@samwho samwho marked this pull request as ready for review September 10, 2024 08:09
@samwho samwho requested a review from a team as a code owner September 10, 2024 08:09
@samwho samwho requested review from adrinr and mike12345567 and removed request for a team September 10, 2024 08:09
Copy link
Collaborator

@mike12345567 mike12345567 left a comment

Choose a reason for hiding this comment

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

LGTM - massive improvement!

@samwho samwho requested a review from adrinr September 10, 2024 09:51
Copy link
Collaborator

@adrinr adrinr left a comment

Choose a reason for hiding this comment

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

Nice to have this cleaner and cleaner every day :D

packages/server/src/integrations/googlesheets.ts Outdated Show resolved Hide resolved
@samwho samwho merged commit ff67cae into master Sep 10, 2024
12 checks passed
@samwho samwho deleted the budi-8579-issue-with-google-sheets branch September 10, 2024 10:07
@github-actions github-actions bot locked and limited conversation to collaborators Sep 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
firestorm Data/Infra/Revenue Team size/xl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants