Skip to content
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.

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

ON-2719 - Add additional POST workflow route necessary to #106

Merged
merged 2 commits into from Feb 4, 2016
Merged

ON-2719 - Add additional POST workflow route necessary to #106

merged 2 commits into from Feb 4, 2016

Conversation

jimturnquist
Copy link
Contributor

ON-2719 - Add additional POST workflow route necessary to support node refresh workflow.

@benbp

@zyoung51
Copy link
Contributor

Are additional unit tests required for this?

@benbp
Copy link
Contributor

benbp commented Jan 28, 2016

Yeah, tests would be nice, though given my comment in #107 this may end up just being a quick fix for 1.1. @jimturnquist hit me up on slack and I can walk you through our unit testing approach if you need it/help you whip up a simple test for this.

var graph = { name: 'foobar' };
taskGraphRunner.runTaskGraph.resolves(graph);

return helper.request().post('/api/1.1/workflows')

Choose a reason for hiding this comment

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

'helper' is not defined.

@jimturnquist
Copy link
Contributor Author

@benbp @heck @zyoung51 Additional unit test has been added for POST /api/1.1/workflows

@heckj
Copy link
Member

heckj commented Feb 3, 2016

👍 Thanks Jim!

return helper.request().post('/api/1.1/workflows')
.send(graph)
.expect('Content-Type', /^application\/json/)
.expect(200, graph);
Copy link
Contributor

Choose a reason for hiding this comment

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

A check to ensure runTaskGraph was called may be a nice addition

@zyoung51
Copy link
Contributor

zyoung51 commented Feb 3, 2016

Could use additional checking in unit tests and addition of functional tests at: https://github.com/RackHD/RackHD/blob/master/test/tests/workflows_tests.py, but the code looks fine to merge as is. +1.

@benbp
Copy link
Contributor

benbp commented Feb 4, 2016

👍

benbp added a commit that referenced this pull request Feb 4, 2016
ON-2719 - Add additional POST workflow route necessary to
@benbp benbp merged commit d4e59c9 into RackHD:release/1.1.0-branch Feb 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants