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.

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

Ordered cli tests #375

Closed
wants to merge 2 commits into from
Closed

Ordered cli tests #375

wants to merge 2 commits into from

Conversation

ndegory
Copy link
Contributor

@ndegory ndegory commented Oct 24, 2016

fixes #374

  • add a step field in the yaml file
  • sort the map entries after unmarshaling the yaml file

how to check:

  • swarm start
  • go test ./cmd/amp/cli
  • if tests are passing, that's great. If not, check in the logs that 08-restart-stack.yml is executed in same order as specified

@ndegory ndegory mentioned this pull request Oct 24, 2016
Closed
@ndegory
Copy link
Contributor Author

ndegory commented Oct 24, 2016

btw, tests are failing, because test 08 is using a command that does not exist (amp stack restart stack1)

bquenin
bquenin reviewed Oct 24, 2016
View reviewed changes
cmd/amp/cli/cli_test.go
for _, k := range commandMap {
sortedCmdSpecs = append(sortedCmdSpecs, k)
}
sort.Sort(CommandSpecByStep(sortedCmdSpecs))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think MapSlices would do the job of keeping the order of the map. Check https://godoc.org/gopkg.in/yaml.v2#MapSlice

@subfuzion
Copy link
Contributor

Hold off on this PR while we discuss the issue. See: #374 (comment).

@neha-viswanathan neha-viswanathan mentioned this pull request Oct 25, 2016
Merged
@subfuzion
Copy link
Contributor

There has been no further discussion on #374, so closing this PR for now, and we'll proceed to implement the changes that were proposed.

@subfuzion subfuzion closed this Oct 26, 2016
@ndegory ndegory mentioned this pull request Oct 28, 2016
Merged
@ndegory ndegory deleted the ordered-cli-tests branch November 8, 2016 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bquenin bquenin bquenin left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Non reproducibility of CLI tests
3 participants
@ndegory @subfuzion @bquenin