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

Add dugite tests #4

Merged
merged 6 commits into from
Oct 23, 2023
Merged

Add dugite tests #4

merged 6 commits into from
Oct 23, 2023

Conversation

confused-Techie
Copy link
Member

This PR builds off of #3 as the changes were easier to implement and test once that was done.

This PR adds some testing of functions that actually use dugite. This is done so that we can be sure changes like #2 are not breaking anything, and we know they work accordingly.

@confused-Techie
Copy link
Member Author

Also a note, I'd recommend we review and merge #3 prior to checking this one out, since without those changes in core this PR is rather cumbersome to review

Copy link
Member

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

This is a "request changes" review, but I like what I see and it gives me an idea for how to do this reproducibly with no issues. Thank you for getting this far with it this quickly!

test/index.test.js Outdated Show resolved Hide resolved
test/fixtures/getDiff.fixture.txt Outdated Show resolved Hide resolved
@confused-Techie
Copy link
Member Author

@DeeDeeG And done, what do we think? Appreciate you noticing how this would bite us later

Copy link
Member

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

-it('tests dugite functionality')

All tests coming back positive! (😄)

Thanks again for this! Feels very thorough to help reassure the dugite bump is going to go smoothly!

@confused-Techie
Copy link
Member Author

Thanks for the approval and your consistent reviews!

But of course, feels good to be able to look at some passing specs and have fully confidence! Lets get it merged, then get the dugite bump updated

@confused-Techie confused-Techie merged commit 86292b1 into master Oct 23, 2023
1 check passed
@confused-Techie confused-Techie deleted the add-dugite-tests branch October 23, 2023 05:59
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