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

Create unit tests for Core #1787

Merged
merged 2 commits into from
Aug 9, 2024
Merged

Conversation

cvegagimenez
Copy link
Contributor

  • Migrate go test to helm unittest
  • Migrate Trivy existing unit tests to helm unittest
  • Add unit tests for Core

@cvegagimenez cvegagimenez force-pushed the feat/add-core-tests branch 2 times, most recently from 6e91857 to 42000ea Compare July 9, 2024 15:40
- Migrate `go test` to `helm unittest`
- Migrate Trivy existing unit tests to `helm unittest`
- Add unit tests for Core

Signed-off-by: Carlos Vega <carlos.vega@dynatrace.com>
path: data.GC_TIME_WINDOW_HOURS
value: "2"

- it: GcTimeWindowHours
Copy link
Collaborator

Choose a reason for hiding this comment

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

line 105, GcTimeWindowHours may need to be changed to a different value, as it is used in line 95 and not related to its context.

path: data.CACHE_EXPIRE_HOURS
value: "3"

- it: CuotaUpdate
Copy link
Collaborator

Choose a reason for hiding this comment

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

CuotaUpdate has a typo here.

Comment on lines 26 to 35
- it: PodAnnotations
set:
core:
podAnnotations:
test.annotation: test-annotation
template: templates/core/core-dpl.yaml
asserts:
- equal:
path: spec.template.metadata.annotations["test.annotation"]
value: test-annotation
Copy link
Collaborator

Choose a reason for hiding this comment

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

line 26 - line35 is a duplicate of line 15 - line24

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the duplicated tests and renamed the wrong ones.

- notExists:
path: data.HARBOR_ADMIN_PASSWORD

- it: ExistingDBSecret
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe change this ExistingDBSecret to ExistingExternalDBSecret?

- notExists:
path: spec.type

- it: ExposeTLSEnabled
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since ExposeTLSEnabled can be misunderstood as expose.tls.enabled, maybe change it to InternalTLSEnabled?

path: spec.ports[0].name
value: https-web

- it: ExposeTLSDisabled
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since ExposeTLSDisabled can be misunderstood as expose.tls.enabled, maybe change it to InternalTLSDisabled?

Comment on lines 8 to 9
registry:
enabled: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't have registry.enabled configuration in values.yaml. Remove it?

- Remove duplicated test PodAnnotations
- Rename test ArtifactPullAsyncFlushDuration

Signed-off-by: Carlos Vega <carlos.vega@dynatrace.com>
Copy link
Collaborator

@MinerYang MinerYang left a comment

Choose a reason for hiding this comment

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

lgtm

@zyyw zyyw merged commit 499b55d into goharbor:main Aug 9, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants