-
Notifications
You must be signed in to change notification settings - Fork 634
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
Regressions Extra Node Info #15555
Regressions Extra Node Info #15555
Conversation
Instead of using a hard-coded value like 58 got the list of expected nodes names in a variable and then if the test fails log the missing nodes.
UI Smoke TestsTest: success. 11 passed, 0 failed. |
@@ -18,6 +18,8 @@ namespace DynamoCoreWpfTests | |||
class NodeAutoCompleteSearchTests : DynamoTestUIBase | |||
{ | |||
|
|||
private readonly List<string> expectedNodes = new List<string> { "ByFillet", "ByFilletTangentToCurve", "ByGeometry", "ByMinimumVolume", "ByBlendBetweenCurves", "ByTangency", "ByLineAndPoint", "ByJoinedCurves", "ByThickeningCurveNormal", "ByLoft", "ByLoft", "ByLoftGuides", "BySweep", "ByLoft", "ByLoft", "ByRevolve", "BySweep", "BySweep2Rails", "ByLoft", "ByLoft", "ByPatch", "ByRevolve", "ByRuledLoft", "BySweep", "BySweep2Rails", "BuildFromLines", "BuildPipes", "ByExtrude", "ByPlaneLineAndPoint", "ByRevolve", "BySweep", "DoesIntersect", "IsAlmostEqualTo", "DistanceTo", "Intersect", "IntersectAll", "Project", "Project", "ProjectInputOnto", "ProjectInputOnto", "Split", "Trim", "SerializeAsSAB", "ClosestPointTo", "Join", "ByGroupedCurves", "SweepAsSolid", "ExportToSAT", "SweepAsSurface", "LocateSurfacesByLine", "BridgeEdgesToEdges", "BridgeEdgesToFaces", "BridgeFacesToEdges", "BridgeFacesToFaces", "CreateMatch", "ExtrudeEdgesAlongCurve", "ExtrudeFacesAlongCurve", "PullVertices" }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps sorting this list, and the found nodes alphabetically first would make sure we don't encounter any weirdness from results being in different orders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, actually I think you're correct and it does not matter because except
should not be affected by order, it's about sets, not lists.
The helix test suite is still failing on this PR. |
Well this change is just for adding extra info about the missing nodes (just when the the NodeAutocomplete tests are failing) |
Trying to execute the job again due that yesterday was failing due to Helix regressions (not related to this change).
re-triggering the job
Looks like the test failures are not related to this. Merging this to unblock and will create a task to handle the failing helix tests. |
@mjkkirschner could you update your PR with latest changes in this commit, just to see if the NodeAutocomplete tests are still failing and if that is the case get more details about the missing nodes? |
Purpose
Adding code for getting extra info and fix flaky regressions
Instead of using a hard-coded value like 58 got the list of expected nodes names in a variable and then if the test fails log the missing nodes.
Declarations
Check these if you believe they are true
*.resx
filesRelease Notes
Adding code for getting extra info and fix flaky regressions
Reviewers
@QilongTang @mjkkirschner
FYIs