DYN-5733 fix language CLI argument#15739
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-5733
chubakueno
changed the title
UI Smoke TestsTest: success. 11 passed, 0 failed. |
| /// <param name="info">Host analytics info specifying Dynamo launching host related information.</param> | ||
| /// <returns></returns> | ||
| public static DynamoModel MakeCLIModel(string asmPath, string userDataFolder, string commonDataFolder, HostAnalyticsInfo info = new HostAnalyticsInfo()) | ||
| public static DynamoModel PrepareModel( |
There was a problem hiding this comment.
you cannot break these public api signatures of a public class.
There was a problem hiding this comment.
@chubakueno I guess a better option should be creating an overloaded for public static DynamoModel MakeModel which receives string CLIlocale as parameter so in this way you won't need to modify the existing public method (I think default argument is also NOT an option).
There was a problem hiding this comment.
Done, though there are now two(previously there already was one) unused overloads of the methods.
There was a problem hiding this comment.
@chubakueno I can see that now we have three MakeModel() methods and only two of them are calling PrepareModel(), is possible to also use PrepareModel() in the missing one?
There was a problem hiding this comment.
Done. Behavior before was not identical to the other overloads so PrepareModel wasn't an equivalent refactor (no calls to AssignHostPathAndIPathResolver, SetUICulture, OnDetectLanguage, OnRequestUpdateLoadBarStatus), but that MakeModel is only used by integration tests and the lack of those calls is likelier than not to be an unintentional omission so i think it's a good idea that all exhibit the same behavior.
RobertGlobant20
left a comment
RobertGlobant20
left a comment
There was a problem hiding this comment.
LGTM with one comment
| /// <param name="info">Host analytics info specifying Dynamo launching host related information.</param> | ||
| /// <returns></returns> | ||
| public static DynamoModel MakeCLIModel(string asmPath, string userDataFolder, string commonDataFolder, HostAnalyticsInfo info = new HostAnalyticsInfo()) | ||
| public static DynamoModel PrepareModel( |
There was a problem hiding this comment.
@chubakueno I can see that now we have three MakeModel() methods and only two of them are calling PrepareModel(), is possible to also use PrepareModel() in the missing one?
|
Waiting for PR checks |
Finished now, looks like UI Smoke Tests failed even though the only change since successful 6494d89 was changing the method to private. Self Serve executed succesfully. |
|
hi @avidit The test |
It was a timeout issue. I have verified the test case manually. |
|
Hi @QilongTang , seems the automated UI test didn't click the Launch Dynamo button properly because of a flaky UI interaction: https://sharereports.smartbear.com/FFBC06CDBA7D49648DD30D76D06B49A801/result . Rerun the test and it's now fully green. |
|
Thank you @avidit @chubakueno for validation. |
5 participants
Purpose
Fix language CLI
-llocale argument to override user preferences, which is currently not functional even though its documented.New behavior with spanish preferred language:
New behavior with no preferred language (default english):
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Fix CLI
-llocale argument to override user language preferences.Reviewers
@QilongTang
@RobertGlobant20
FYIs
@avidit