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

Issue 110 tests improvements connlist pkg #250

Merged
merged 15 commits into from
Oct 22, 2023

Conversation

shireenf-ibm
Copy link
Contributor

@shireenf-ibm shireenf-ibm commented Oct 18, 2023

issue #110

  • this PR , includes tests improvements to connlist_test.go file
    all test funcs run in parallel, and for each test func, all cases run in parallel too.

  • tried to split TestXXX funcs by "testing cases" to avoid if else conditions in the test functionality

  • this PR does not handle generating/overriding expected output files (prefer to keep it for a separate PR, want to study if it is possible to write a workflow that could be run by go test --override (for example))

@shireenf-ibm shireenf-ibm requested a review from adisos October 18, 2023 08:44
Copy link
Collaborator

@adisos adisos left a comment

Choose a reason for hiding this comment

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

Looks very good!
Added a few comments.

pkg/netpol/connlist/connlist_test.go Outdated Show resolved Hide resolved
pkg/netpol/connlist/connlist_test.go Outdated Show resolved Hide resolved
pkg/netpol/connlist/connlist_test.go Outdated Show resolved Hide resolved
@adisos
Copy link
Collaborator

adisos commented Oct 19, 2023

issue #110

* this PR , includes tests improvements to `connlist_test.go` file
  all test `funcs` run in parallel, and for each `test func, all cases run in parallel too.`

* tried to split `TestXXX` funcs by "testing cases" to avoid `if else` conditions in the test functionality

* this PR does not handle generating/overriding expected output files (prefer to keep it for a separate PR, want to study if it is possible to write a workflow that could be run by `go test --override` (for example))

As for generating/overriding expected output files -- please consider we also need to be able to run this locally, not necessarily in a workflow.

@shireenf-ibm shireenf-ibm requested a review from adisos October 20, 2023 11:36
pkg/netpol/connlist/connlist_test.go Outdated Show resolved Hide resolved
pkg/netpol/connlist/connlist_test.go Outdated Show resolved Hide resolved
pkg/netpol/connlist/connlist_test.go Outdated Show resolved Hide resolved
@shireenf-ibm shireenf-ibm requested a review from adisos October 22, 2023 09:15
@shireenf-ibm shireenf-ibm requested a review from adisos October 22, 2023 12:06
@shireenf-ibm
Copy link
Contributor Author

this commit
fixes the lint issue on master branch

@shireenf-ibm shireenf-ibm merged commit e02d840 into main Oct 22, 2023
@shireenf-ibm shireenf-ibm deleted the issue_110_tests_improvements_connlist_pkg branch October 22, 2023 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants