Skip to content

Conversation

@RobertGlobant20
Copy link
Contributor

Purpose

Cherry-Pick Fix Node Autocomplete Regressions
#14275

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

Cherry-Pick Fix Node Autocomplete Regressions

Reviewers

@QilongTang @reddyashish

FYIs

* Fix Node Autocomplete Regressions

Due that we are adding ZeroTouchNodes during test mode around 9 tests are failing, seems that there are several nodes duplicates so each test needs to be analyzed separately.
Then in this fix I'm reverting the code for adding ZeroTouchNodes and also removing a test that will fail due to this change.

* Fix Node Autocomplete Regressions

Re-adding unit test with Failure category
/// This test will validate that the nodes "Input", "Output", "And", "Or", "Not", "+", "-" appear in the InCanvasSearch results
/// </summary>
[Test]
[Category("UnitTests")]
Copy link
Contributor

Choose a reason for hiding this comment

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

@RobertGlobant20 [Category("UnitTests")] should also be present.

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 searched for Failure category in Dynamo solution and in all the places is only Failure, do you have an example of a test with both categories?
e.g.
image

Copy link
Contributor

@reddyashish reddyashish Aug 18, 2023

Choose a reason for hiding this comment

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

You can just add another category attribute like this:
[Test]
[Category("UnitTests")]
[Category("Failure")]
or like this:
https://github.com/DynamoDS/Dynamo/blob/master/test/DynamoCoreTests/UnitsOfMeasureTests.cs#L1047

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reddyashish ok got your point.
Due that this PR is a cherry-pick, could you approve and merge it? (If I do changes over this branch won't be cherry-picking the commit).
I will do the change in the original branch and then create another cherry-pick PR.

Copy link
Contributor

@QilongTang QilongTang Aug 18, 2023

Choose a reason for hiding this comment

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

@RobertGlobant20 This works, will approve and merge

@QilongTang QilongTang merged commit 415fe3c into DynamoDS:RC2.19.0_master Aug 18, 2023
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.

3 participants