Skip to content

Conversation

ipince
Copy link
Contributor

@ipince ipince commented Jul 25, 2023

Summary

I'm sorry if this steps a bit on your toes :( . I just figured I wanted some unit test before I made the changes to the merging logic, and noticed that if I just refactored a function a little bit, then we could add some unit tests easily at the function level.

I just added 1 unit test here to demonstrate, and will add more later as I implement the bugfix.

How was it tested?

Unit tests

@ipince ipince requested a review from savil July 25, 2023 20:11
Copy link
Collaborator

@savil savil left a comment

Choose a reason for hiding this comment

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

Thanks! This looks great, and nope, this didn't step on my toes at all. Thanks for adding it!

I'll focus on adding testscript tests which are more end-to-end than these golang unit-tests (to minimize conflicts)

@ipince ipince force-pushed the rodrigo/update-test branch from 12c26f2 to b3c6b7d Compare July 26, 2023 17:12
@ipince ipince merged commit 6746296 into main Jul 26, 2023
@ipince ipince deleted the rodrigo/update-test branch July 26, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants