Skip to content

DYN-8353: Allow tests to run against ASM231 #15848

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.

Sign up for GitHub

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

Jump to bottom
Merged
sm6srw merged 9 commits into from
Mar 3, 2025
Merged

DYN-8353: Allow tests to run against ASM231 #15848

sm6srw merged 9 commits into from
Mar 3, 2025

Conversation

@sm6srw
Copy link
Contributor

@sm6srw sm6srw commented Feb 22, 2025 •
edited
Loading

Purpose

This pull request does:

  • allow tests to run against ASM231. The latest release of ASC is installed on the new build machines and we will start to run all tests against ASM231 after we migrate.
  • split DynamoCoreWpfTest into three assemblies. The last 400 tests or so in this assembly started to fail consistently when running against ASM231. It looks like Wpf fails to load resources and then crashes so I guess something is leaking. I have not been able to figure it out so far. I can make it work again if I disable any 30% of the tests in the assembly. I decided to split the assembly into three assembly's for now and it seems to make it very stable. I have seen these crashes before but they have been sporadic.

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

N/A

Reviewers

(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)

(FILL ME IN, optional) Any additional notes to reviewers or testers.

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

@sm6srw sm6srw added the WIP label Feb 22, 2025
github-actions[bot]
github-actions bot reviewed Feb 22, 2025
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8353

sm6srw
sm6srw commented Feb 22, 2025
View reviewed changes
test/DynamoCoreWpfTests/dynamoMenusRedesign.config
Copy link
Contributor Author

@sm6srw sm6srw Feb 22, 2025

Choose a reason for hiding this comment

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

Looks like I lost this file. Need to check.

Copy link
Contributor Author

@sm6srw sm6srw Feb 24, 2025

Choose a reason for hiding this comment

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

This file was not referenced anywhere as far as I can tell.

sm6srw
sm6srw commented Feb 22, 2025
View reviewed changes
src/Dynamo.All.sln
Copy link
Contributor Author

@sm6srw sm6srw Feb 22, 2025

Choose a reason for hiding this comment

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

Linux is back. I need to clean up this file.

@sm6srw sm6srw removed the WIP label Feb 28, 2025
@sm6srw
Copy link
Contributor Author

sm6srw commented Feb 28, 2025

Job running against ASM231 succeeded here: https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/dahlj%252Ftesting_master/61/

@sm6srw
Copy link
Contributor Author

sm6srw commented Feb 28, 2025

This is now ready for review.

@sm6srw sm6srw added the PTAL Please Take A Look 👀 label Feb 28, 2025
zeusongit
zeusongit approved these changes Mar 2, 2025
View reviewed changes
Copy link
Contributor

@zeusongit zeusongit left a comment

Choose a reason for hiding this comment

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

LGTM, although does this mean that we will have to split the tests again as we reach a certain number of tests in a single project?

@sm6srw
Copy link
Contributor Author

sm6srw commented Mar 3, 2025

LGTM, although does this mean that we will have to split the tests again as we reach a certain number of tests in a single project?

I think this is related to WPF in this case. We do have issues with resource leaks that needs to be fixed at some point. But in general, I do think it is better to have a larger number of but smaller test assemblies so we should not be afraid to create new ones.

@sm6srw sm6srw merged commit d541a79 into master Mar 3, 2025
23 checks passed
@sm6srw sm6srw deleted the Prep_for_231 branch March 3, 2025 13:36
sm6srw added a commit that referenced this pull request Mar 4, 2025
@sm6srw
DYN-8353: Allow tests to run against ASM231 (#15848)
09ff426
sm6srw added a commit that referenced this pull request Mar 4, 2025
@sm6srw
Cherry pick #15848
ef81228
sm6srw added a commit that referenced this pull request Mar 4, 2025
@sm6srw
DYN-8353: Allow tests to run against ASM231 (#15848)
646103c
sm6srw added a commit that referenced this pull request Mar 4, 2025
@sm6srw
DYN-8353: Allow tests to run against ASM231 (#15848)
b1e5294
sm6srw added a commit that referenced this pull request Mar 4, 2025
@sm6srw
DYN-8353: Allow tests to run against ASM231 (#15848)
e6fb099
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@zeusongit zeusongit zeusongit approved these changes

Assignees

No one assigned

Labels

PTAL Please Take A Look 👀

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sm6srw @zeusongit