fork with binary files

[pelikhan]

This fix uses the fork head commit instead of the upstream commit when creating the PR tree. A number of tests fail as a result but I'm unsure whether that's expected or something broke.

Tested here: https://github.com/microsoft/jacdac/pull/932

• [x] flip assertions in fork tests (expected <-> actual value)
• [x] use fork head instead of parent head when creating trees of changes
• [x] adding missing create-fork-blob test variant
• [ ] patching tests (they probably need proper recording)

[gr2m]

If you confirmed it working as expected, I'd say update the tests and let's merge it?

[pelikhan]

It does break a few other tests. I haven’t tested all scenarios. Unclear if the test are wrong.

---

From: Gregor Martynus @.> Sent: Friday, March 11, 2022 4:00:52 PM To: gr2m/octokit-plugin-create-pull-request @.> Cc: Peli de Halleux @.>; Author @.> Subject: Re: [gr2m/octokit-plugin-create-pull-request] fork with binary files (PR #86)

If you confirmed it working as expected, I'd say update the tests and let's merge it?

— Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgr2m%2Foctokit-plugin-create-pull-request%2Fpull%2F86%23issuecomment-1065677924&data=04%7C01%7Cjhalleux%40microsoft.com%7Cc17a33ce30a84ffbd37d08da03bb5fd9%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637826400573220561%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=gmZPJsDZ0vzM3Ic5OIrSJAepVIx3pJ70iSvJH3fzUxY%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAA73QKI4YBNQG766VE355N3U7PNDJANCNFSM5QNE3DMQ&data=04%7C01%7Cjhalleux%40microsoft.com%7Cc17a33ce30a84ffbd37d08da03bb5fd9%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637826400573220561%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=QurkWva8EnJn7o2O%2Bfd9hGKxz3ivd4UmwtKi2VgmYWc%3D&reserved=0. You are receiving this because you authored the thread.Message ID: @.***>

[gr2m]

Hmm at first sight the error with test/use-fork.test.ts seems legit. If I read it right, it tries to load commits from gr2m instead of hipstersmoothie. I don't have time right now to investigate further myself. But if you do and share your findings and come to a conclusion how to fix the fork workflow for binary files without breaking existing workflows, I'm all for it

The CONTRIBUTING.md file has instructions how to update the test fixtures in case that's helpful

[pelikhan]

It may just be that the test output is confusing... It seems that we have the arguments backwards in the "expect" call!

In such case, the failure makes sense. We are trying to work on the fork side of things instead of the parent repo. IMO, the test need updating... but I'm not a git wizard nor plan to become one. Would be good to get more eyes on this.

[gr2m]

Could you help resolving the conflicts? I'd be happy to review and merge then