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

Ensures that task list is a python list #561

Merged
merged 1 commit into from Jul 8, 2015
Merged

Conversation

ivotron
Copy link
Contributor

@ivotron ivotron commented Jul 7, 2015

@ceph-jenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
http://jenkins.ceph.com//job/teuthology-pull-requests/1627/
Test FAILed.

@ivotron ivotron changed the title Ensures that task list is a python list DNM: Ensures that task list is a python list Jul 7, 2015
@ceph-jenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
http://jenkins.ceph.com//job/teuthology-pull-requests/1629/
Test PASSed.

@ivotron ivotron changed the title DNM: Ensures that task list is a python list Ensures that task list is a python list Jul 7, 2015
@@ -161,9 +161,12 @@ def validate_tasks(config):
# return the default value for tasks
return []

msg = 'Expecting (bullet) list of tasks instead of unordered map'
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe print out config['tasks'] here as well? Maybe: msg = "Expected a list of tasks and instead got: {0}".format(config['tasks'])

Copy link
Contributor

Choose a reason for hiding this comment

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

Or even just log.info it somewhere before the assertion.

@ceph-jenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
http://jenkins.ceph.com//job/teuthology-pull-requests/1630/
Test PASSed.

@ceph-jenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
http://jenkins.ceph.com//job/teuthology-pull-requests/1631/
Test PASSed.

for task in config['tasks']:
msg = ('kernel installation shouldn be a base-level item, not part ' +
'of the tasks list')
msg = ("kernel installation shouldn't be a base-level item, not part " +
Copy link
Member

Choose a reason for hiding this comment

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

I think this should actually read "should be a base-level item"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thought it was "shouldn't" . fixed below

@ceph-jenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
http://jenkins.ceph.com//job/teuthology-pull-requests/1633/
Test PASSed.

@zmc
Copy link
Member

zmc commented Jul 7, 2015

This looks good to me; maybe squashing some commits (at least the most recent one) would be good

@ivotron
Copy link
Contributor Author

ivotron commented Jul 7, 2015

how does squashing work for PRs on github? can I force an update to this
branch? or should I create a new branch?

On Tue, Jul 7, 2015 at 2:57 PM Zack Cerza notifications@github.com wrote:

This looks good to me; maybe squashing some commits (at least the most
recent one) would be good


Reply to this email directly or view it on GitHub
#561 (comment).

@zmc
Copy link
Member

zmc commented Jul 7, 2015

Yeah, you can force push this branch

@ceph-jenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
http://jenkins.ceph.com//job/teuthology-pull-requests/1634/
Test PASSed.

@ivotron
Copy link
Contributor Author

ivotron commented Jul 8, 2015

done. switched back to single quotes

@ceph-jenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
http://jenkins.ceph.com//job/teuthology-pull-requests/1638/
Test PASSed.

  * Modifies test for task list validation
  * Shows contents of config['tasks'] for error msg
  * Properly checks distinct failure conditions
  * Fixes typo on kernel task AssertionError
@ivotron
Copy link
Contributor Author

ivotron commented Jul 8, 2015

OK. Done with last modifications. Thanks!

@ceph-jenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
http://jenkins.ceph.com//job/teuthology-pull-requests/1640/
Test PASSed.

dmick added a commit that referenced this pull request Jul 8, 2015
Ensures that task list is a python list

Reviewed-by: Dan Mick <dmick@redhat.com>
Reviewed-by: Zack Cerza <zcerza@redhat.com>
Reviewed-by: Andrew Schoen <aschoen@redhat.com>
@dmick dmick merged commit 6e53646 into ceph:master Jul 8, 2015
@ivotron ivotron deleted the wip-12226 branch July 8, 2015 23:21
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