Skip to content

[API-321] Migrate tests for blast writes#342

Merged
schottra merged 6 commits intomainfrom
comms-blast-tests
Aug 29, 2025
Merged

[API-321] Migrate tests for blast writes#342
schottra merged 6 commits intomainfrom
comms-blast-tests

Conversation

@schottra
Copy link
Copy Markdown
Contributor

This is a followup to #338 to migrate the quite sizable blasts test file. Source is here: https://github.com/AudiusProject/audius-protocol/blob/ab73d979f98e151ac0aa408c7a82c105c3b6355e/comms/discovery/rpcz/chat_blast_test.go#L760

  • Migrated everything to pgx patterns
  • Split the file into multiple isolated tests instead of one huge test
  • Using fixtures instead of manual inserts for setup data
  • Removed a lot of the 'in the past' time variables in favor of just using time.Now() for operations that are sequential
  • Updated the version of getNewBlasts in the comms directory to match the one we use in the get endpoint. We are being intentionally WET here.
  • Fixed an issue with upgrading blasts to chats where the timestamp that we assign to the new message would be in the wrong timezone if run on a machine not set to UTC. In practice, this won't break on CI or production, but it does break when running on a local machine 😢

As a bonus, also cleaned up the tests from the previous PR to match the patterns here.

Comment thread api/comms/chat.go
($1, $2, $3, $4, $5)
on conflict do nothing
`, trashid.BlastMessageID(blast.BlastID, params.ChatID), params.ChatID, blast.FromUserID, blast.CreatedAt, blast.BlastID)
`, messageId, params.ChatID, blast.FromUserID, blast.CreatedAt.UTC(), blast.BlastID)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note here the usage of .UTC(). On my machine, when running this, the location() on this time.Time was being set to Local and that was causing us to insert messages in the past. Since chat_blast uses a timestamptz column type and chat_message uses a plain timestamp column type, pgx semantics when inserting will cause it to try and map from the timezone in the incoming variable to the timezone of the machine running postgres.
Kind of gnarly 😬

Comment thread api/comms/chat_test_queries.go Outdated
Comment thread api/comms/chat_test_queries.go Outdated
Comment thread api/comms_mutate_test.go
})
}

/* TODO:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why remove?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't think writing them would add a ton of value for the amount of time it would take. Can be persuaded though :-p

@schottra schottra merged commit df019b2 into main Aug 29, 2025
5 checks passed
@schottra schottra deleted the comms-blast-tests branch August 29, 2025 18:52
schottra added a commit that referenced this pull request Aug 29, 2025
* origin/main:
  [API-321] Migrate tests for blast writes (#342)
  Update query for best selling (#347)
  Remove membersChange24hrPercent (#346)
  [QA-2301] Fix blockees req (#345)
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.

2 participants