-
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix old bugs/wrong behavior in
applyIfNeedsToApply & related logic
previously, in `gitStackedRebase`, there were 3 invocations of `applyIfNeedsToApply`. there are still 3, but what differs is: previously, the 3rd would be invoked if and only if the `config.autoApplyIfNeeded` was true, i.e., it was NOT mandatory. however, the 1st invocation, which also is NOT mandatory, did not have such guardrail, and, if the `config.autoApplyIfNeeded` was false, would prompt the user with the common message: `need to --apply before continuing. proceed? [Y/n/(a)lways] ` this is wrong, because this (1st) invocation is not mandatory, meanwhile the message makes it look like it is, thus confusing the user & their longer term understanding of when an --apply is actually needed. to further add to the confusion, the 1st invocation worked correctly apart from the message - it (1st invoc) was not made mandatory, i.e. would not stop the program execution if user did not allow. --- now, there's a clear distinction of mandatory vs not, and `applyIfNeedsToApply` works accordingly to it. related parts were also cleaned up. also, created a test for this -- yay! Signed-off-by: Kipras Melnikovas <[email protected]>
- Loading branch information
Showing
5 changed files
with
209 additions
and
99 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| import assert from "assert"; | ||
|
|
||
| import Git from "nodegit"; | ||
|
|
||
| import { configKeys } from "../config"; | ||
| import { gitStackedRebase } from "../git-stacked-rebase"; | ||
| import { humanOpChangeCommandOfNthCommitInto } from "../humanOp"; | ||
| import { askQuestion__internal, editor__internal, noEditor } from "../internal"; | ||
|
|
||
| import { setupRepo } from "./util/setupRepo"; | ||
| import { question, Questions } from "../util/createQuestion"; | ||
|
|
||
| export async function applyTC() { | ||
| await integration__git_stacked_rebase_exits_if_apply_was_needed_but_user_disallowed(); | ||
| } | ||
|
|
||
| /** | ||
| * create a scenario where an apply is needed, and disallow it - GSR should exit. | ||
| */ | ||
| async function integration__git_stacked_rebase_exits_if_apply_was_needed_but_user_disallowed() { | ||
| const { initialBranch, common, commitsInLatest, config } = await setupRepo(); | ||
|
|
||
| /** | ||
| * ensure autoApplyIfNeeded is disabled | ||
| */ | ||
| config.setBool(configKeys.autoApplyIfNeeded, Git.Config.MAP.FALSE); | ||
|
|
||
| /** | ||
| * force modify history, so that an apply will be needed | ||
| */ | ||
| await gitStackedRebase(initialBranch, { | ||
| ...common, | ||
| [editor__internal]: ({ filePath }) => { | ||
| humanOpChangeCommandOfNthCommitInto("drop", { | ||
| filePath, // | ||
| commitSHA: commitsInLatest[2], | ||
| }); | ||
| }, | ||
| }); | ||
|
|
||
| // TODO: assert that the "needs to apply" mark is set | ||
|
|
||
| console.log("performing 2nd rebase, expecting it to throw."); | ||
|
|
||
| const threw: boolean = await didThrow(() => | ||
| /** | ||
| * perform the rebase again - now that an apply is marked as needed, | ||
| * and autoApplyIfNeeded is disabled, | ||
| * we should get prompted to allow the apply. | ||
| */ | ||
| gitStackedRebase(initialBranch, { | ||
| ...common, | ||
| ...noEditor, | ||
| [askQuestion__internal]: (q, ...rest) => { | ||
| if (q === Questions.need_to_apply_before_continuing) { | ||
| return "n"; | ||
| } | ||
|
|
||
| return question(q, ...rest); | ||
| }, | ||
| }) | ||
| ); | ||
|
|
||
| assert.deepStrictEqual( | ||
| threw, | ||
| true, | ||
| `expected 2nd invocation of rebase to throw, because user did not allow to perform a mandatory --apply.\nbut threw = ${threw} (expected true).` | ||
| ); | ||
| } | ||
|
|
||
| export async function didThrow(fn: Function): Promise<boolean> { | ||
| try { | ||
| await fn(); | ||
| return false; | ||
| } catch (_e) { | ||
| return true; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters