Skip to content

DYN-1706: Graphs with Automatic Run Type honor RunEnabled settings#9641

Merged
QilongTang merged 8 commits intoDynamoDS:masterfrom
reddyashish:master
Apr 12, 2019
Merged

DYN-1706: Graphs with Automatic Run Type honor RunEnabled settings#9641
QilongTang merged 8 commits intoDynamoDS:masterfrom
reddyashish:master

Conversation

@reddyashish
Copy link
Collaborator

@reddyashish reddyashish commented Apr 10, 2019

Purpose

This PR is to make sure that the graphs that are set to automatic run type honor the RunEnabled settings. The RunEnabled flag is checked in the RequestRun function as well as in the ResetEngine function.
JIRA: https://jira.autodesk.com/browse/DYN-1706

Declarations

Check these if you believe they are true

  • The code base 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.

Reviewers

@QilongTang @mjkkirschner @aparajit-pratap

@mjkkirschner
Copy link
Member

@reddyashish this is kind of worrisome:
#4674

did you check that this issue that @benglin was working on does not reoccur after your change?

@reddyashish
Copy link
Collaborator Author

Yes, this was just a draft and let's discuss about this issue in the standup.

@QilongTang
Copy link
Contributor

QilongTang commented Apr 10, 2019

@reddyashish @mjkkirschner I think checking the node value in this case is not easy unless I am missing pieces. I tried myself and with @reddyashish together, node value is not guaranteed to be accurate, same with homeworkspace.evaluationCount. I expect the following test code to pass but it did not. I am wondering if we need to move this to a Model Test instead of a view test.

            var openPath = Path.Combine(GetTestDirectory(ExecutingDirectory), @"core\RunSettings.dyn");
            ViewModel.OpenCommand.Execute(openPath);

            var ws = Model.CurrentWorkspace as HomeWorkspaceModel;

            AssertEvaluationCount(ws, 0);
            ws.RequestRun();
            // Still in Manual mode so will not run
            AssertEvaluationCount(ws, 0);

            ws.RunSettings.RunType = RunType.Automatic;
            // Setting to automatic will triger run right away
            AssertEvaluationCount(ws, 1);

            ws.RunSettings.RunEnabled = false;
            ws.RequestRun();
            // This should not run
            AssertEvaluationCount(ws, 1);

            ws.RunSettings.RunEnabled = true;
            ws.RequestRun();
            // This should run
            AssertEvaluationCount(ws, 2);

@reddyashish reddyashish removed the WIP label Apr 11, 2019
@QilongTang
Copy link
Contributor

LGTM

@QilongTang QilongTang merged commit 504f53b into DynamoDS:master Apr 12, 2019
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.

4 participants