This repository was archived by the owner on Oct 30, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 189
Test IncludeGenerated #381
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
0.15.0 | ||
0.16.0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
apiVersion: apps/v1 | ||
kind: Deployment | ||
metadata: | ||
name: deployment | ||
spec: | ||
selector: | ||
matchLabels: | ||
name: deployment | ||
template: | ||
metadata: | ||
labels: | ||
name: deployment | ||
spec: | ||
containers: | ||
- name: container | ||
image: scratch |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,5 +9,5 @@ spec: | |
- protocol: TCP | ||
port: 80 | ||
targetPort: 9376 | ||
clusterIP: 10.0.171.239 | ||
type: LoadBalancer | ||
clusterIP: 10.96.0.1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The service.yml ClusterIP was invalid ("The Service "test-service" is invalid: spec.clusterIP: Invalid value: "10.0.171.239": provided IP is not in the valid range. The range of valid IPs is 10.96.0.0/16"). This doesn't affect tests or anything. I just happened to try to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for catching that! |
||
type: LoadBalancer |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Is this the normal convention that the package test have a different name than the package itself? I've not done this before and I'm wondering if I should have been...
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 remember is something to do with blackbox/whitebox testing 💭. I think when we call this a different name, we would have to import
k8sinternal
in order to get access to the rest of the code.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.
Exactly what Brian said! It depends what you want to test.
If you use the same package as the code you're testing (whitebox), you have access to the private methods in the package and can test those.
If you use the special
_test
package (normally Go won't let you have multiple package names in the same directory, so appending_test
to the package is a special case), you can test your code as if you were a consumer of the package (blackbox).In this case we're not testing any private methods so using the "test" package is kind of a cheat to get out of import cycle problems. Alternatively we could move the functions that cause the import cycle into a new package and import that from both
k8sinternal
andtest
.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.
TYVM for the explainer!