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

Replace reflect.DeepEqual with cmp.Diff in tests #2268

Closed
tenzen-y opened this issue Mar 2, 2024 · 10 comments · Fixed by #2289
Closed

Replace reflect.DeepEqual with cmp.Diff in tests #2268

tenzen-y opened this issue Mar 2, 2024 · 10 comments · Fixed by #2289
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed kind/feature

Comments

@tenzen-y
Copy link
Member

tenzen-y commented Mar 2, 2024

/kind feature

Describe the solution you'd like
[A clear and concise description of what you want to happen.]
To improve visibility, we should replace reflect.DeepEqual with cmp.Diff in all test cases: https://github.com/kubeflow/katib/blob/master/docs/developer-guide.md#go-development

Here is an example for using cmp.Diff:

if diff := cmp.Diff(tc.wantConfig, kc.RuntimeConfig.SuggestionConfigs); len(diff) != 0 {
t.Errorf("Unexpected SuggestionConfigs (-want,+got):\n%s", diff)
}

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]


Love this feature? Give it a 👍 We prioritize the features with the most 👍

@tenzen-y
Copy link
Member Author

tenzen-y commented Mar 2, 2024

/good-first-issue

Copy link

@tenzen-y:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@google-oss-prow google-oss-prow bot added good first issue Good for newcomers help wanted Extra attention is needed labels Mar 2, 2024
@tariq-hasan
Copy link
Contributor

@tenzen-y I would like to take on this ticket.

@tenzen-y
Copy link
Member Author

tenzen-y commented Mar 2, 2024

@tenzen-y I would like to take on this ticket.

Sure, Feel free to assign this yourself with "/assign" comment.

@tariq-hasan
Copy link
Contributor

/assign

@Mu-Magdy
Copy link

Hi @tenzen-y

Is this issue solved or I can work on it?

@tenzen-y
Copy link
Member Author

@tariq-hasan Are you still working on this?

@tariq-hasan
Copy link
Contributor

I am working on it. I will open a new PR.

Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@tenzen-y
Copy link
Member Author

/remove-lifecycle stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment