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

Support visualtree verification for Controls #126

Merged
merged 8 commits into from Mar 15, 2019

Conversation

licanhua
Copy link
Contributor

@licanhua licanhua commented Dec 20, 2018

Fix #228

Provide a baseline version(master copy) of visualtree for controls, then verify it for future changes.
The pull request provide functionality:

  1. dump visualtree and store it to local
  2. package the 'master copy' xml files to /master of testapp
    3, compare visualtree with 'master copy'
  3. if inconsistency found, dump the master copy and visualtree to files. eg: NavigationViewVisualTreeTests_VerifyVisualTreeForNavView_Dark-8.xml and NavigationViewVisualTreeTests_VerifyVisualTreeForNavView_Dark-8.xml.orig
  4. Log MasterFile to MusicLibrary or LocalFolder. By default(AlwaysLogMasterFile=false), It logs the master files to LocalFolder. You can change the setting by set AlwaysLogMasterFile = true. After you run the tests and test case fails, the master files are put into MusicLibrary. Finally you can copy master files to master/ directory and check in with code.
  5. Provide Filter and PropertyValueTranslator to allow user to customize the visualtree output.

Some rule to avoid conflicts:

  1. master name format: [masterFileNamePrefix].xml or [masterFileNamePrefix]_[theme]-[apiversion].xml.
  2. master file searching: If running os is RS5, the api version = 7, then first look if [masterFileNamePrefix]-7.xml exists, if not, try [masterFileNamePrefix]-6.xml... [masterFileNamePrefix]-2.xml, and finally [masterFileNamePrefix].xml or null

Restrictions:

  1. Didn't support HC theme

@licanhua licanhua requested review from jevansaks, ranjeshj, kmahone and a team December 20, 2018 21:46
Copy link
Contributor

@ranjeshj ranjeshj left a comment

Choose a reason for hiding this comment

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

Do we need a full baseline/compare or can we have a bit more targeted approach and avoid the baseline files ?

test/MUXControlsTestApp/VisualTreeTestBase.cs Outdated Show resolved Hide resolved
MUXControls.sln Outdated Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Outdated Show resolved Hide resolved
dev/CommonStyles/APITests/CommonStylesTests.cs Outdated Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Outdated Show resolved Hide resolved
dev/CommonStyles/APITests/CommonStylesTests.cs Outdated Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Outdated Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Outdated Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Outdated Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Outdated Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Outdated Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Outdated Show resolved Hide resolved
@jevansaks jevansaks added this to Needs review in Controls Dev Feb 21, 2019
@licanhua licanhua added the auto merge This PR will be merged once all checks pass label Mar 9, 2019
All,
None
}
class VisualTreeLog
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment describing the purpose of this class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
class VisualTreeLog
// This class is used for internal debug purpose only. We don't have a flag to control log level in UnitTesting.Logging.
// Like debug information, it's too big and we should not show to customer. It will provide more information if you make small change on LogDebugInfo like this
// public static void LogDebugInfo(string debugInfo) { Log.Comment(info); }
class VisualTreeLog

{
return _testResult.ToString();
}
public void DumpAndVerifyVisualTree(UIElement root, string masterFilePrefix, string messageOnError = null)
Copy link
Member

Choose a reason for hiding this comment

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

Add a newline between these functions

}
}

class VisualTreeOutputCompare
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment about the purpose of this class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
class VisualTreeOutputCompare
// A simple string diff class to provide readable text to show the difference of two strings. Here is a example of output.
// + This is only in A
// - This is only in B
class VisualTreeOutputCompare

test/TestAppUtils/VisualTreeDumper.cs Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Show resolved Hide resolved
test/TestAppUtils/VisualTreeDumper.cs Show resolved Hide resolved
}
}

private class TestExecution
Copy link
Contributor

Choose a reason for hiding this comment

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

new file for each class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's nested internal and private class and I don't expect other one to use it.

public string GetTestResult()
{
return _testResult.ToString();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

newline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
}
}

Verify.Fail(helper.GetTestResult(), "Test Failed");
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

TestExecution sounds like something coming from wex. What does this do? Why cant this just be a method instead of a class ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All functions in VisualTreeTestHelper are static, and it has a long list of parameter like (UIElement root, string masterFilePrefix, Theme theme = Theme.None, IPropertyValueTranslator translator = null, IFilter filter = null, IVisualTreeLogger logger = null).
TestExecution just make all these parameter to a class and let the class to manage them.
TestExecution is also a central place to manage the test result, we may dump and verify for dark, then we want test continue even if the first verification fails. then we only need to check if helper.HasError for all verification.


class VisualTreeOutputCompare
{
public VisualTreeOutputCompare(string a, string b)
Copy link
Contributor

Choose a reason for hiding this comment

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

why a class ? why not just
string CompareStrings(string a, string b)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will not make change here. In the first implementation, we need to use existing diff in os repo, and DiffPlex in MUX. so it's implemented as a class.

Controls Dev automation moved this from Needs review to Reviewer approved Mar 15, 2019
@msft-github-bot msft-github-bot merged commit 8c8c116 into master Mar 15, 2019
Controls Dev automation moved this from Reviewer approved to Done Mar 15, 2019
@msft-github-bot msft-github-bot deleted the user/canli/visualtree2 branch March 15, 2019 17:43
msft-github-bot pushed a commit that referenced this pull request Mar 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merge This PR will be merged once all checks pass
Projects
No open projects
Controls Dev
  
Done
Development

Successfully merging this pull request may close these issues.

Proposal: Support Visualtree dumping and 'master file' testing
7 participants