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 with trailing spaces #282

Open
stavros-k opened this issue Jan 17, 2024 · 12 comments
Open

Issue with trailing spaces #282

stavros-k opened this issue Jan 17, 2024 · 12 comments

Comments

@stavros-k
Copy link
Contributor

stavros-k commented Jan 17, 2024

One of the weird failures is this one:

EDIT: Look at the bottom first

(TLDR, only issue is when a fail message is printed with a trailing space and test suite does not have that trailing space, test will fail, but you can't really see the reason, see the first test below)

Suggestion: TrimSpace on both fail message and expected message

 FAIL  cnpg scheduled backup validation test	library/common-test/tests/cnpg/scheduled_backup_validation_test.yaml
	- should fail with invalid backupOwnerReference in scheduledBackups backup

		- asserts[0] `failedTemplate` fail
			Template:	common-test/templates/common.yaml
			DocumentIndex:	0
			Expected to equal:
				CNPG Scheduled Backup - Expected [backupOwnerReference] in [backups.scheduledBackups] entry to be one of [none, self, cluster], but got [invalid]
			Actual:
				CNPG Scheduled Backup - Expected [backupOwnerReference] in [backups.scheduledBackups] entry to be one of [none, self, cluster], but got [invalid]

Every other test failure is regarding isNull check, eg

 FAIL  init container data test (upgrade)	library/common-test/tests/initContainer/data_upgrade_test.yaml
	- should NOT generate render init container

		- asserts[2] `isNull` fail
			Template:	common-test/templates/common.yaml
			DocumentIndex:	0
			Path:	spec.template.spec.initContainers expected to NOT exists

Cloning https://github.com/truecharts/library-charts and running run_common_tests.sh should have all tests passing.

 helm version 
version.BuildInfo{Version:"v3.13.3", GitCommit:"c8b948945e52abba22ff885446a1486cb5fd3474", GitTreeState:"clean", GoVersion:"go1.20.11"}

❯ helm plugin list 
NAME    	VERSION	DESCRIPTION                                                                         
unittest	0.4.1  	Unit test for helm chart in YAML with ease to keep your chart functional and robust.

If there is any additional info that you need, please let me know

@stavros-k
Copy link
Contributor Author

stavros-k commented Jan 17, 2024

So the first issue that fails even if the expected == actual is working fine up to 0.3.4 but fails on 0.3.5
and the isNull issue works fine up to 0.3.1 but fails from 0.3.2 (same happens with notExists)

Hope that helps narrow it down

@tewfik-ghariani
Copy link
Contributor

You have to use the newly added isNullOrEmpty / exists validators ( and of course isNotNullOrEmpty / notExists )

These replace isNull/isEmpty and their opposing counterparts when the key is rendered but with a null or empty value

In case the key is never rendered, isNull is replaced with notExists

Ref: #134

@stavros-k
Copy link
Contributor Author

You have to use the newly added isNullOrEmpty / exists validators ( and of course isNotNullOrEmpty / notExists )

These replace isNull/isEmpty and their opposing counterparts when the key is rendered but with a null or empty value

In case the key is never rendered, isNull is replaced with notExists

Ref: #134

Ah rereading the "action" for each of the above, I see the issue now and seems like its working after updating the matcher.

But still the failedTemplate doesn't make sense to me! Do you see anything weird there?

@stavros-k
Copy link
Contributor Author

Found the issue!

In the helm file the error message had a space in the end.
Yaml test file trims the space at the end, I fixed it in my end, but might be a good call to trim whitespace at the end when comparing?

Feel free to close if you don't think thats a good idea!

@sandzhaj
Copy link

I also updated from 0.3.4 to 0.4.1 and have some issues:

 Meta.App.Deployment      meta/tests/deployment_test.yaml
        - Correct Render
                Error: template: ipodso-appchart/templates/t_deployment.yaml:70:30: executing "ipodso-appchart/templates/t_deployment.yaml" at <include "deployment.secretsVolumes" .>: error calling include: template: ipodso-appchart/charts/common-library/templates/_deployment.tpl:179:14: executing "deployment.secretsVolumes" at <.Values.deployment.fluentBit.enabled>: nil pointer evaluating interface {}.enabled

I noticed that for some reason, my included template deployment.secretsVolumes in t_deployment.yaml don't see .Values.deployment.fluentBit, while main template could see that.

I am sure that is not an issue of my templates, because helm template command works good with my values, helm unittest 0.3.4 also works good.

I'm actually don't understand about space that @stavros-k was talking about. I removed all empty lines in test file, but no result

@stavros-k
Copy link
Contributor Author

In my case the error message had a trailing space.

In you case is seems that you tried to access .enabled but fluentBit is either null or {}
if you do a check like if and .Values.deployment.fluentBit .Values.deployment.fluentBit.enabled
Check that you are not running helm 3.9.4 or below, as those versions did not shortcircuiting

@camalot
Copy link

camalot commented Jan 30, 2024

In my situation:

  • helm version: 3.8.1
  • helm-unittest: 0.3.6 tests pass
  • helm-unittes: 0.4.1 tests fail

tests/configmap_test.yaml

---
suite: Test configmap
templates:
- "my-service-configmap.yaml"
tests:
  - it: should override all values for one of two pods
    set:
      my-service:
        some-property:
          base_url: "https://bar.value.domain/" # sets SERVICE_BASE_URL in the configmap
    asserts:
      - equal:
          path: data.SERVICE_BASE_URL
          value: "https://bar.value.domain/"
        documentIndex: 0

values.yaml

  my-service:
    some-property:
      base_url: https://foo.value.domain:443

output

- asserts[10] `equal` fail
    Template:	service-chart/templates/my-configmap.yaml
    DocumentIndex:	0
    Path:	data.SERVICE_BASE_URL
    Expected to equal:
      https://bar.value.domain/
    Actual:
      https://foo.value.domain:443
    Diff:
      --- Expected
      +++ Actual
      @@ -1,2 +1,2 @@
      -https://bar.value.domain/
      +https://foo.value.domain:443

@tewfik-ghariani
Copy link
Contributor

@camalot This looks like a problem with the "my-service-configmap.yaml" template itself not the unit test, the property is not actually overriding the base url

@camalot
Copy link

camalot commented Jan 31, 2024

@camalot This looks like a problem with the "my-service-configmap.yaml" template itself not the unit test, the property is not actually overriding the base url

@tewfik-ghariani that implies that the tests prior to using 0.4.0 would pass, when they were actually failing? Because this test (and the others) passed before 0.4.0.

Also, in the multiple regions that my-service-configmap is deployed in it has the correct overridden values (changes per region, which is why there is a test for it).

I also only gave a single example from this one chart that had failing tests. Every test in this chart is failing. We did not run against our other charts. We just pinned to use 0.3.6.

I can run against a chart with the new version and get more details. The example that i gave was pretty redacted to remove anything specific to the organization.

@stavros-k
Copy link
Contributor Author

Can you share a minimal reproduction folder containing files that can actually reproduce it?

I used all versions since 0.3.0 against 1200unit tests and didn't have a single test fail (apart from 3-4 mentioned in this issue, that was actually my error), on a what I believe a complex library chart.

@camalot
Copy link

camalot commented Jan 31, 2024

I think there is something with the tests and/or the chart itself...

I am trying to figure out what the team was trying to test, and the expectation. I am acting more a middle man here.

The part that does scare me though is the fact that the tests pass as "expected" on 0.3.6, but fail on 0.4.x.

Once I determine what is happening fully with this chart and the tests I will try to create a reproduction folder for it.

@stavros-k stavros-k changed the title Upgrade from 0.3.0 to 0.4.1 and some tests fail Issue with trailing spaces Feb 10, 2024
@sandzhaj
Copy link

I also updated from 0.3.4 to 0.4.1 and have some issues:

btw
0.4.4 works

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

No branches or pull requests

4 participants