Skip to content

Repair Functional Tests: test for confirm on and off#335

Merged
jeschu1 merged 1 commit intomicrosoft:masterfrom
jeschu1:repair_functional_tests
Oct 8, 2018
Merged

Repair Functional Tests: test for confirm on and off#335
jeschu1 merged 1 commit intomicrosoft:masterfrom
jeschu1:repair_functional_tests

Conversation

@jeschu1
Copy link
Member

@jeschu1 jeschu1 commented Oct 4, 2018

Enrich existing tests to include confirm off

Per @kewillford 's request. This change should run quickly and give us added security.


this.Enlistment.TryMountGVFS().ShouldEqual(false, "Repair without confirm should not fix the enlistment");

this.Enlistment.Repair(true);
Copy link
Member

Choose a reason for hiding this comment

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

Helper method for these three lines?

this.Enlistment.TryMountGVFS().ShouldEqual(false, "GVFS shouldn't mount when HEAD is corrupt");

this.Enlistment.Repair();
this.Enlistment.Repair(false);
Copy link
Member

Choose a reason for hiding this comment

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

We usually use named parameters for constant values like
this.Enlistment.Repair(confirm: false);

"repair --confirm \"" + this.enlistmentRoot + "\"",
failOnError: true);
string confirmArg = confirm == true ? "--confirm " : string.Empty;
if (confirm)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you should be checking this bool otherwise the repair will not be ran when false. We still want to run it just without the --confirm

this.CallGVFS(
"repair --confirm \"" + this.enlistmentRoot + "\"",
failOnError: true);
string confirmArg = confirm == true ? "--confirm " : string.Empty;
Copy link
Member

Choose a reason for hiding this comment

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

Don't need the == true here.

public void NoFixesNeeded()
{
this.Enlistment.UnmountGVFS();

Copy link
Contributor

Choose a reason for hiding this comment

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

Another style nit: I think there are an excessive number of blank lines in these tests. It's hard to read code when each line of code is followed by a blank line :-)

@jeschu1 jeschu1 force-pushed the repair_functional_tests branch 2 times, most recently from 45c106f to f00be38 Compare October 4, 2018 15:37
@jeschu1 jeschu1 force-pushed the repair_functional_tests branch from f00be38 to b627ceb Compare October 4, 2018 16:19
Enrich existing tests to include confirm off
@jeschu1 jeschu1 force-pushed the repair_functional_tests branch from b627ceb to 46f9bc5 Compare October 4, 2018 17:21
@jeschu1 jeschu1 closed this Oct 5, 2018
@jeschu1 jeschu1 reopened this Oct 5, 2018
@jeschu1 jeschu1 merged commit 1c6a49f into microsoft:master Oct 8, 2018
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.

4 participants