-
Notifications
You must be signed in to change notification settings - Fork 376
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
Update documentation #1136
Update documentation #1136
Conversation
Thanks for your PR. The following commands are available:
|
e5d0c78
to
1378881
Compare
1378881
to
9b70e7c
Compare
ci/README.md
Outdated
|
||
We run 4 different categories of tests as part of CI: | ||
* **unit tests**: most Go packages for Antrea include some unit tests written | ||
using the [Go `testing`] package. Unit tests typically rely on mock testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder why [Go testing
], not [Go testing
] or Go [testing
] ? I think it looks strange.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to Go [testing
]
ci/README.md
Outdated
* **unit tests**: most Go packages for Antrea include some unit tests written | ||
using the [Go `testing`] package. Unit tests typically rely on mock testing | ||
to isolate the package being tested, and abstract platform-specific | ||
functionality hidden behing interfaces. When adding a new package or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
functionality hidden behing interfaces. When adding a new package or | |
functionality hidden behind interfaces. When adding a new package or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks
ci/README.md
Outdated
typically exercise multiple Go packages to ensure that they are working | ||
correctly together. In the case of Antrea, integration tests may create an | ||
actual OVS bridge, which is why they depend on the OVS daemons running, and | ||
are typically run inside a Docker image, even on Linux. Write integrations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are typically run inside a Docker image, even on Linux. Write integrations | |
are typically run inside a Docker image, even on Linux. Write integration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks
ci/README.md
Outdated
also written using the [Go `testing`] package. Unlike the two previous test | ||
categories, these assume that Antrea is running on an actual cluster and | ||
require a kubeconfig file as well as SSH access to the cluster | ||
Nodes. Instructions on how to run these tests, including how to setup a local |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These lines seem not wrapped strictly, e.g. the above line has enough space for "Node."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's funny, The auto-wrap in emacs refused to place Nodes on the same line as cluster. I had to do it manually...
* add missing linters * emphasize that new test cases should be added as part of new contributions and include more details about the different categories of tests we support; this is related to antrea-io#989
9b70e7c
to
03f1179
Compare
Thanks for the reviews, I addressed all comments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/skip-all |
* Update documentation * add missing linters * emphasize that new test cases should be added as part of new contributions and include more details about the different categories of tests we support; this is related to antrea-io#989 * Address review comments
contributions and include more details about the different categories
of tests we support; this is related to
Obtain "passing" CII Best Practices badge #989