Skip to content

DYN-6607 - Input/Output Node - part 1 FIX#15037

Merged
QilongTang merged 3 commits intoDynamoDS:masterfrom
dnenov:definedata-master-fix
Mar 22, 2024
Merged

DYN-6607 - Input/Output Node - part 1 FIX#15037
QilongTang merged 3 commits intoDynamoDS:masterfrom
dnenov:definedata-master-fix

Conversation

@dnenov
Copy link
Collaborator

@dnenov dnenov commented Mar 20, 2024

Purpose

Fixes a number of failing tests blocking master-15.
Ignored tests are already fixed in the follow-up PR #15022 (probably a mix-up when cherry-picking).

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

  • fixes a number of tests causing
  • some tests are already fixed due to upcoming changes in a follow-up PR - these were temporarily ignored

Reviewers

@QilongTang
@mjkkirschner

FYIs

@saintentropy
@twastvedt

@dnenov
definedata hotfix
ecaa876 
- fixes a number of tests causing
- some tests are already fixed due to upcoming changes in a follow-up PR - these were temporarily ignored
@QilongTang
Copy link
Contributor

QilongTang commented Mar 21, 2024

I cant find the pull request validation job for this PR.. So I started https://master-15.jenkins.autodesk.com/view/DYN/job/DYN-DevCI_Self_Service/1415/ for you

@QilongTang
Copy link
Contributor

QilongTang commented Mar 21, 2024

One regression from the job: Dynamo.Tests.Configuration.PreferenceSettingsTests.TestImportCopySettings

@QilongTang QilongTang added this to the 3.1 milestone Mar 21, 2024
dnenov added 2 commits March 22, 2024 09:42
@dnenov
Merge remote-tracking branch 'upstream/master' into definedata-master…
ba3aa2c 
…-fix
@dnenov
TestImportCopySettings fix
6811454 
- fixed TestImportCopySettings by adding the new Preference property to the test settings file
@dnenov
Copy link
Collaborator Author

dnenov commented Mar 22, 2024

One regression from the job: Dynamo.Tests.Configuration.PreferenceSettingsTests.TestImportCopySettings

Thank you, @QilongTang ! I don't know how this went in - it was a new property added to the Preferences - TemplateFilePath. I added that to the settings test file fixing the test.

@dnenov dnenov closed this Mar 22, 2024
@dnenov dnenov reopened this Mar 22, 2024
@github-actions
Copy link

github-actions bot commented Mar 22, 2024 •
edited
Loading

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

QilongTang
QilongTang reviewed Mar 22, 2024
View reviewed changes
test/settings/DynamoSettings-NewSettings.xml
<BackupFiles>
<string>C:\Users\user\Documents\dynamofile.dyn</string>
</BackupFiles>
<TemplateFilePath>C:\ProgramData\Dynamo\Dynamo Core\templates\en-US</TemplateFilePath>
Copy link
Contributor

@QilongTang QilongTang Mar 22, 2024

Choose a reason for hiding this comment

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

@zeusongit It looks like now this property is serialized always? Although the value could be empty. Please confirm

Copy link
Collaborator Author

@dnenov dnenov Mar 22, 2024

Choose a reason for hiding this comment

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

Hi @QilongTang - I am not the author of the preference property, I assume it's a new addition.

Dynamo/src/DynamoCore/Configuration/PreferenceSettings.cs

Line 949 in 97e2d46

TemplateFilePath = string.Empty;

The test picks it up as the number of default preferences coming from PreferenceSettings.cs was different then the one specified in the test file. I just added the entry in the test file to account for the new property and fix the test.

Copy link
Collaborator Author

@dnenov dnenov Mar 22, 2024

Choose a reason for hiding this comment

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

I don't know if this answers the question, but I am not sure if the property should or shouldn't be serialized.

@QilongTang QilongTang merged commit 97e2d46 into DynamoDS:master Mar 22, 2024
twastvedt added a commit that referenced this pull request Apr 10, 2024
@twastvedt
Revert "DYN-6607 - Input/Output Node - part 1 FIX (#15037)"
4838e90 
This reverts commit 97e2d46.

# Conflicts:
#	test/Tools/NodeDocumentationMarkdownGeneratorTests/MarkdownGeneratorCommandTests.cs
@twastvedt twastvedt mentioned this pull request Apr 10, 2024
Merged
9 tasks
QilongTang pushed a commit that referenced this pull request Apr 15, 2024
@twastvedt
DYN-4654 Revert IO node development for 3.1 (#15110)
83e388a 
* Revert "DYN-6607 - Input/Output Node - part 1 FIX (#15037)"

This reverts commit 97e2d46.

# Conflicts:
#	test/Tools/NodeDocumentationMarkdownGeneratorTests/MarkdownGeneratorCommandTests.cs

* Revert "DYN-6607 - Input/Output Node - part 1 (#14987)"

This reverts commit c90f105.
@twastvedt twastvedt mentioned this pull request Apr 15, 2024
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@QilongTang QilongTang QilongTang approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.1

Development

Successfully merging this pull request may close these issues.

2 participants

@dnenov @QilongTang