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

endpoint: move TestReadEpsFromDirNames to pkg/endpoint #8997

Merged

Conversation

ianvernon
Copy link
Member

@ianvernon ianvernon commented Aug 22, 2019

The test was previously in `daemon`, and necessitated leaking a lot of
Endpoint-related state. Instead, refactor the test into the Endpoint package.
Do not regenerate the endpoints needed for testing; instead, write their
headerfiles using `writeHeaderfile`, since that is the operation which
is needed to be performed in order to perform the test to read the serialized
Endpoints from their corresponding headerfiles.

Signed-off by: Ian Vernon ian@cilium.io


This change is Reviewable

@ianvernon ianvernon added pending-review kind/cleanup This includes no functional changes. labels Aug 22, 2019
@ianvernon ianvernon requested a review from a team August 22, 2019 00:06
@ianvernon ianvernon requested review from a team as code owners August 22, 2019 00:06
@ianvernon ianvernon changed the title endpoint: move \TestReadEpsFromDirNames\ to \pkg/endpoint\ endpoint: move TestReadEpsFromDirNames to pkg/endpoint Aug 22, 2019
@ianvernon ianvernon force-pushed the pr/ianvernon/move-TestReadEPsFromDirNames-to-endpoint-pkg branch from 2f0dace to 6b62f1e Compare August 22, 2019 00:07
@ianvernon
Copy link
Member Author

test-me-please

@ianvernon ianvernon added this to In progress in Endpoint Cleanup via automation Aug 22, 2019
@ianvernon ianvernon added this to the 1.7 milestone Aug 22, 2019
@ianvernon ianvernon force-pushed the pr/ianvernon/move-TestReadEPsFromDirNames-to-endpoint-pkg branch from 6b62f1e to b6d0081 Compare August 22, 2019 00:12
@ianvernon
Copy link
Member Author

test-me-please

@coveralls
Copy link

coveralls commented Aug 22, 2019

Coverage Status

Coverage increased (+0.009%) to 44.085% when pulling 852d91d on pr/ianvernon/move-TestReadEPsFromDirNames-to-endpoint-pkg into af710fd on master.

@ianvernon ianvernon force-pushed the pr/ianvernon/move-TestReadEPsFromDirNames-to-endpoint-pkg branch from b6d0081 to bcd40bf Compare August 22, 2019 00:36
@ianvernon
Copy link
Member Author

test-me-please

@@ -31,7 +31,6 @@ import (
"github.com/cilium/cilium/pkg/labels"
"github.com/cilium/cilium/pkg/policy"
"github.com/cilium/cilium/pkg/policy/trafficdirection"

Copy link
Contributor

Choose a reason for hiding this comment

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

This is unintended, I think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

The test was previously in `daemon`, and necessitated leaking a lot of
Endpoint-related state. Instead, refactor the test into the Endpoint package.
Do not regenerate the endpoints needed for testing; instead, write their
headerfiles using `writeHeaderfile`, since that is the operation which
is needed to be performed in order to perform the test to read the serialized
Endpoints from their corresponding headerfiles.

Signed-off by: Ian Vernon <ian@cilium.io>
@ianvernon ianvernon force-pushed the pr/ianvernon/move-TestReadEPsFromDirNames-to-endpoint-pkg branch from bcd40bf to 852d91d Compare August 22, 2019 14:29
@ianvernon
Copy link
Member Author

test-me-please

@ianvernon ianvernon merged commit 3528c61 into master Aug 22, 2019
Endpoint Cleanup automation moved this from In progress to Done Aug 22, 2019
@ianvernon ianvernon deleted the pr/ianvernon/move-TestReadEPsFromDirNames-to-endpoint-pkg branch August 22, 2019 16:20
@aanm aanm added the release-note/misc This PR makes changes that have no direct user impact. label Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants