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

Fix flaky unit tests in resources package #438

Merged

Conversation

vdemeester
Copy link
Member

Sorting the slices from GetOutputSteps and GetInputSteps in test,
to make sure we have a consistent ordering of the slice.

As maps have non-deterministic order in Go, and as we don't sort in
the actual methods (I don't think we need to have a deterministic
slice), the compared slice may not be in the same order. Sorting it by
name fixes that.

Related to #434 (comment)

/cc @bobcatfish @shashwathi

Signed-off-by: Vincent Demeester vdemeest@redhat.com

Sorting the slices from `GetOutputSteps` and `GetInputSteps` in test,
to make sure we have a consistent ordering of the slice.

As maps have non-deterministic order in Go, and as we don't sort in
the actual methods (I don't think we need to have a deterministic
slice), the compared slice may not be in the same order. Sorting it by
name fixes that.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
@googlebot googlebot added the cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit label Jan 25, 2019
@knative-prow-robot knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 25, 2019
@nader-ziada
Copy link
Member

/lgtm

/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 25, 2019
@knative-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pivotal-nader-ziada, vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [pivotal-nader-ziada]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 25, 2019
@knative-prow-robot knative-prow-robot merged commit a561dd3 into tektoncd:master Jan 25, 2019
@vdemeester vdemeester deleted the fix-flaky-unit-resources branch January 25, 2019 14:55
@bobcatfish
Copy link
Collaborator

oh shoot another one! 😅 i ran into this over in #415 (comment) too!

i solved this in a slightly different manner if you are interested :D 5f6625d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants