ronantakizawa/codereview-bench huggingface dataset:
Repository: helm/helm
File: pkg/chart/v2/util/dependencies.go (Go)

Code before:
	if err := processDependencyEnabled(c, v, ""); err != nil {
		return err
	}
	return processDependencyImportValues(c, true)
}

// processDependencyConditions disables charts based on condition path value in values
func processDependencyConditions(reqs []*chart.Dependency, cvals Values, cpath string) {
	if reqs == nil {
		return
	}
	for _, r := range reqs {
		for _, c := range strings.Split(strings.TrimSpace(r.Condition), ",") {
			if len(c) > 0 {
				// retrieve value
				vv, err := cvals.PathValue(cpath + c)
				if err == nil {
					// if not bool, warn
					if bv, ok := vv.(bool); ok {
						r.Enabled = bv
						break
					}
					slog.Warn("returned non-bool value", "path", c, "chart", r.Name)
				} else if _, ok := err.(ErrNoValue); !ok {
					// this is a real error
					slog.Error("pathValue returned error", slog.Any("err", err))
				}
			}
		}
	}
}

// processDependencyTags disables charts based on tags in values
func processDependencyTags(reqs []*chart.Dependency, cvals Values) {
	if reqs == nil {
		return
	}
	vt, err := cvals.Table("tags")
	if err != nil {
		return
	}
	for _, r := range reqs {
		var hasTrue, hasFalse bool
		for _, k := range r.Tags {
			if b, ok := vt[k]; ok {
				// if not bool, warn
				if bv, ok := b.(bool); ok {
					if bv {
						hasTrue = true
					} else {
						hasFalse = true

Review comment (suggestion):
The original was a Warning. So, this should be `slog.Warn` instead of `Error`.

Code after:
	if err := processDependencyEnabled(c, v, ""); err != nil {
		return err
	}
	return processDependencyImportValues(c, true)
}

// processDependencyConditions disables charts based on condition path value in values
func processDependencyConditions(reqs []*chart.Dependency, cvals Values, cpath string) {
	if reqs == nil {
		return
	}
	for _, r := range reqs {
		for _, c := range strings.Split(strings.TrimSpace(r.Condition), ",") {
			if len(c) > 0 {
				// retrieve value
				vv, err := cvals.PathValue(cpath + c)
				if err == nil {
					// if not bool, warn
					if bv, ok := vv.(bool); ok {
						r.Enabled = bv
						break
					}
					slog.Warn("returned non-bool value", "path", c, "chart", r.Name)
				} else if _, ok := err.(ErrNoValue); !ok {
					// this is a real error
					slog.Warn("the method PathValue returned error", slog.Any("error", err))
				}
			}
		}
	}
}

// processDependencyTags disables charts based on tags in values
func processDependencyTags(reqs []*chart.Dependency, cvals Values) {
	if reqs == nil {
		return
	}
	vt, err := cvals.Table("tags")
	if err != nil {
		return
	}
	for _, r := range reqs {
		var hasTrue, hasFalse bool
		for _, k := range r.Tags {
			if b, ok := vt[k]; ok {
				// if not bool, warn
				if bv, ok := b.(bool); ok {
					if bv {
						hasTrue = true
					} else {
						hasFalse = true
Repository: helm/helm
File: pkg/chart/v2/util/dependencies.go (Go)

Code before:
		cvals, err = MergeValues(c, nil)
	} else {
		cvals, err = CoalesceValues(c, nil)
	}
	if err != nil {
		return err
	}
	b := make(map[string]interface{})
	// import values from each dependency if specified in import-values
	for _, r := range c.Metadata.Dependencies {
		var outiv []interface{}
		for _, riv := range r.ImportValues {
			switch iv := riv.(type) {
			case map[string]interface{}:
				child := iv["child"].(string)
				parent := iv["parent"].(string)

				outiv = append(outiv, map[string]string{
					"child":  child,
					"parent": parent,
				})

				// get child table
				vv, err := cvals.Table(r.Name + "." + child)
				if err != nil {
					slog.Error("importValues missing table from chart", "chart", r.Name, "value", err)
					continue
				}
				// create value map from child to be merged into parent
				if merge {
					b = MergeTables(b, pathToMap(parent, vv.AsMap()))
				} else {
					b = CoalesceTables(b, pathToMap(parent, vv.AsMap()))
				}
			case string:
				child := "exports." + iv
				outiv = append(outiv, map[string]string{
					"child":  child,
					"parent": ".",
				})
				vm, err := cvals.Table(r.Name + "." + child)
				if err != nil {
					slog.Error("importValues missing table", slog.Any("err", err))
					continue
				}
				if merge {
					b = MergeTables(b, vm.AsMap())
				} else {
					b = CoalesceTables(b, vm.AsMap())
				}
			}

Review comment (suggestion):
The original was a warning so this should use `slog.Warn` instead

Code after:
		cvals, err = MergeValues(c, nil)
	} else {
		cvals, err = CoalesceValues(c, nil)
	}
	if err != nil {
		return err
	}
	b := make(map[string]interface{})
	// import values from each dependency if specified in import-values
	for _, r := range c.Metadata.Dependencies {
		var outiv []interface{}
		for _, riv := range r.ImportValues {
			switch iv := riv.(type) {
			case map[string]interface{}:
				child := iv["child"].(string)
				parent := iv["parent"].(string)

				outiv = append(outiv, map[string]string{
					"child":  child,
					"parent": parent,
				})

				// get child table
				vv, err := cvals.Table(r.Name + "." + child)
				if err != nil {
					slog.Warn("ImportValues missing table from chart", "chart", r.Name, "error", err)
					continue
				}
				// create value map from child to be merged into parent
				if merge {
					b = MergeTables(b, pathToMap(parent, vv.AsMap()))
				} else {
					b = CoalesceTables(b, pathToMap(parent, vv.AsMap()))
				}
			case string:
				child := "exports." + iv
				outiv = append(outiv, map[string]string{
					"child":  child,
					"parent": ".",
				})
				vm, err := cvals.Table(r.Name + "." + child)
				if err != nil {
					slog.Warn("ImportValues missing table", slog.Any("error", err))
					continue
				}
				if merge {
					b = MergeTables(b, vm.AsMap())
				} else {
					b = CoalesceTables(b, vm.AsMap())
				}
			}
Repository: helm/helm
File: pkg/plugin/installer/vcs_installer.go (Go)

Code before:
	semvers := getSemVers(refs)

	// Sort semver list
	sort.Sort(sort.Reverse(semver.Collection(semvers)))
	for _, v := range semvers {
		if constraint.Check(v) {
			// If the constraint passes get the original reference
			ver := v.Original()
			slog.Debug("setting to version", "version", ver)
			return ver, nil
		}
	}

	return "", errors.Errorf("requested version %q does not exist for plugin %q", i.Version, i.Repo.Remote())
}

// setVersion attempts to checkout the version
func (i *VCSInstaller) setVersion(repo vcs.Repo, ref string) error {
	slog.Debug("setting version", "version", i.Version)
	return repo.UpdateVersion(ref)
}

// sync will clone or update a remote repo.
func (i *VCSInstaller) sync(repo vcs.Repo) error {
	if _, err := os.Stat(repo.LocalPath()); os.IsNotExist(err) {
		slog.Debug("cloning", "source", repo.Remote(), "destination", repo.LocalPath())
		return repo.Get()
	}
	slog.Debug("updating", "remote", repo.Remote())
	return repo.Update()
}

// Filter a list of versions to only included semantic versions. The response
// is a mapping of the original version to the semantic version.
func getSemVers(refs []string) []*semver.Version {
	var sv []*semver.Version
	for _, r := range refs {
		if v, err := semver.NewVersion(r); err == nil {
			sv = append(sv, v)
		}
	}
	return sv
}


Review comment (suggestion):
On line 163 there is the same key/value pair but the key is "remote". Can we have the same key name everywhere? Please also keep it consistent with line 92.

The idea is that people can use log tools to filter on keys and values. We want to be consistent with the key names to aide these tools.

Code after:
	semvers := getSemVers(refs)

	// Sort semver list
	sort.Sort(sort.Reverse(semver.Collection(semvers)))
	for _, v := range semvers {
		if constraint.Check(v) {
			// If the constraint passes get the original reference
			ver := v.Original()
			slog.Debug("setting to version", "version", ver)
			return ver, nil
		}
	}

	return "", errors.Errorf("requested version %q does not exist for plugin %q", i.Version, i.Repo.Remote())
}

// setVersion attempts to checkout the version
func (i *VCSInstaller) setVersion(repo vcs.Repo, ref string) error {
	slog.Debug("setting version", "version", i.Version)
	return repo.UpdateVersion(ref)
}

// sync will clone or update a remote repo.
func (i *VCSInstaller) sync(repo vcs.Repo) error {
	if _, err := os.Stat(repo.LocalPath()); os.IsNotExist(err) {
		slog.Debug("cloning", "source", repo.Remote(), "destination", repo.LocalPath())
		return repo.Get()
	}
	slog.Debug("updating", "source", repo.Remote(), "destination", repo.LocalPath())
	return repo.Update()
}

// Filter a list of versions to only included semantic versions. The response
// is a mapping of the original version to the semantic version.
func getSemVers(refs []string) []*semver.Version {
	var sv []*semver.Version
	for _, r := range refs {
		if v, err := semver.NewVersion(r); err == nil {
			sv = append(sv, v)
		}
	}
	return sv
}
Repository: helm/helm
File: pkg/engine/lookup_func.go (Go)

Code before:
		return nil, false, errors.Wrapf(err, "unable to get apiresource from unstructured: %s", gvk.String())
	}
	gvr := schema.GroupVersionResource{
		Group:    apiRes.Group,
		Version:  apiRes.Version,
		Resource: apiRes.Name,
	}
	intf, err := dynamic.NewForConfig(config)
	if err != nil {
		slog.Error("unable to get dynamic client", slog.Any("error", err))
		return nil, false, err
	}
	res := intf.Resource(gvr)
	return res, apiRes.Namespaced, nil
}

func getAPIResourceForGVK(gvk schema.GroupVersionKind, config *rest.Config) (metav1.APIResource, error) {
	res := metav1.APIResource{}
	discoveryClient, err := discovery.NewDiscoveryClientForConfig(config)
	if err != nil {
		slog.Error("unable to create discovery client", slog.Any("error", err))
		return res, err
	}
	resList, err := discoveryClient.ServerResourcesForGroupVersion(gvk.GroupVersion().String())
	if err != nil {
		slog.Error("unable to retrieve resource list", "list", gvk.GroupVersion().String(), "error", err)
		return res, err
	}
	for _, resource := range resList.APIResources {
		// if a resource contains a "/" it's referencing a subresource. we don't support subresource for now.
		if resource.Kind == gvk.Kind && !strings.Contains(resource.Name, "/") {
			res = resource
			res.Group = gvk.Group
			res.Version = gvk.Version
			break
		}
	}
	return res, nil
}


Review comment (suggestion):
Instead of `list` for the key, it should be `GroupVersion`.

Code after:
		return nil, false, errors.Wrapf(err, "unable to get apiresource from unstructured: %s", gvk.String())
	}
	gvr := schema.GroupVersionResource{
		Group:    apiRes.Group,
		Version:  apiRes.Version,
		Resource: apiRes.Name,
	}
	intf, err := dynamic.NewForConfig(config)
	if err != nil {
		slog.Error("unable to get dynamic client", slog.Any("error", err))
		return nil, false, err
	}
	res := intf.Resource(gvr)
	return res, apiRes.Namespaced, nil
}

func getAPIResourceForGVK(gvk schema.GroupVersionKind, config *rest.Config) (metav1.APIResource, error) {
	res := metav1.APIResource{}
	discoveryClient, err := discovery.NewDiscoveryClientForConfig(config)
	if err != nil {
		slog.Error("unable to create discovery client", slog.Any("error", err))
		return res, err
	}
	resList, err := discoveryClient.ServerResourcesForGroupVersion(gvk.GroupVersion().String())
	if err != nil {
		slog.Error("unable to retrieve resource list", "GroupVersion", gvk.GroupVersion().String(), "error", err)
		return res, err
	}
	for _, resource := range resList.APIResources {
		// if a resource contains a "/" it's referencing a subresource. we don't support subresource for now.
		if resource.Kind == gvk.Kind && !strings.Contains(resource.Name, "/") {
			res = resource
			res.Group = gvk.Group
			res.Version = gvk.Version
			break
		}
	}
	return res, nil
}
Repository: helm/helm
File: pkg/registry/client.go (Go)

Code before:
	if err != nil {
		return nil, err
	}

	configDescriptor, err := oras.PushBytes(ctx, memoryStore, ConfigMediaType, configData)
	if err != nil {
		return nil, err
	}

	layers := []ocispec.Descriptor{chartDescriptor}
	var provDescriptor ocispec.Descriptor
	if operation.provData != nil {
		provDescriptor, err = oras.PushBytes(ctx, memoryStore, ProvLayerMediaType, operation.provData)
		if err != nil {
			return nil, err
		}

		layers = append(layers, provDescriptor)
	}

	// sort layers for determinism, similar to how ORAS v1 does it
	sort.Slice(layers, func(i, j int) bool {
		return layers[i].Digest < layers[j].Digest
	})

	ociAnnotations := generateOCIAnnotations(meta, operation.creationTime)

	manifestDescriptor, err := c.tagManifest(ctx, memoryStore, ref, configDescriptor,
		layers, ociAnnotations)
	if err != nil {
		return nil, err
	}

	repository, err := remote.NewRepository(parsedRef.String())
	if err != nil {
		return nil, err
	}
	repository.PlainHTTP = c.plainHTTP
	repository.Client = c.authorizer

	manifestDescriptor, err = oras.ExtendedCopy(ctx, memoryStore, parsedRef.String(), repository, parsedRef.String(), oras.DefaultExtendedCopyOptions)
	if err != nil {
		return nil, err
	}

	chartSummary := &descriptorPushSummaryWithMeta{
		Meta: meta,
	}
	chartSummary.Digest = chartDescriptor.Digest.String()
	chartSummary.Size = chartDescriptor.Size
	result := &PushResult{

Review comment (suggestion):
I think the `parsedRef, err := newReference(ref)` above should be removed

Code after:
	if err != nil {
		return nil, err
	}

	configDescriptor, err := oras.PushBytes(ctx, memoryStore, ConfigMediaType, configData)
	if err != nil {
		return nil, err
	}

	layers := []ocispec.Descriptor{chartDescriptor}
	var provDescriptor ocispec.Descriptor
	if operation.provData != nil {
		provDescriptor, err = oras.PushBytes(ctx, memoryStore, ProvLayerMediaType, operation.provData)
		if err != nil {
			return nil, err
		}

		layers = append(layers, provDescriptor)
	}

	// sort layers for determinism, similar to how ORAS v1 does it
	sort.Slice(layers, func(i, j int) bool {
		return layers[i].Digest < layers[j].Digest
	})

	ociAnnotations := generateOCIAnnotations(meta, operation.creationTime)

	manifestDescriptor, err := c.tagManifest(ctx, memoryStore, configDescriptor,
		layers, ociAnnotations, parsedRef)
	if err != nil {
		return nil, err
	}

	repository, err := remote.NewRepository(parsedRef.String())
	if err != nil {
		return nil, err
	}
	repository.PlainHTTP = c.plainHTTP
	repository.Client = c.authorizer

	manifestDescriptor, err = oras.ExtendedCopy(ctx, memoryStore, parsedRef.String(), repository, parsedRef.String(), oras.DefaultExtendedCopyOptions)
	if err != nil {
		return nil, err
	}

	chartSummary := &descriptorPushSummaryWithMeta{
		Meta: meta,
	}
	chartSummary.Digest = chartDescriptor.Digest.String()
	chartSummary.Size = chartDescriptor.Size
	result := &PushResult{
Repository: helm/helm
File: pkg/registry/client.go (Go)

Code before:
	}

	u.Path = fmt.Sprintf("%s:%s", registryReference.Repository, tag)

	return u, err
}

// tagManifest prepares and tags a manifest in memory storage
func (c *Client) tagManifest(ctx context.Context, memoryStore *memory.Store,
	ref string, configDescriptor ocispec.Descriptor, layers []ocispec.Descriptor,
	ociAnnotations map[string]string) (ocispec.Descriptor, error) {

	manifest := ocispec.Manifest{
		Versioned:   specs.Versioned{SchemaVersion: 2},
		Config:      configDescriptor,
		Layers:      layers,
		Annotations: ociAnnotations,
	}

	manifestData, err := json.Marshal(manifest)
	if err != nil {
		return ocispec.Descriptor{}, err
	}

	parsedRef, err := newReference(ref)
	if err != nil {
		return ocispec.Descriptor{}, err
	}

	return oras.TagBytes(ctx, memoryStore, ocispec.MediaTypeImageManifest,
		manifestData, parsedRef.String())
}


Review comment (suggestion):
This is the only actual change here I think, using `parsedRef.String()` instead of `ref` although the tests are nice.

Code after:
	}

	u.Path = fmt.Sprintf("%s:%s", registryReference.Repository, tag)

	return u, err
}

// tagManifest prepares and tags a manifest in memory storage
func (c *Client) tagManifest(ctx context.Context, memoryStore *memory.Store,
	configDescriptor ocispec.Descriptor, layers []ocispec.Descriptor,
	ociAnnotations map[string]string, parsedRef reference) (ocispec.Descriptor, error) {

	manifest := ocispec.Manifest{
		Versioned:   specs.Versioned{SchemaVersion: 2},
		Config:      configDescriptor,
		Layers:      layers,
		Annotations: ociAnnotations,
	}

	manifestData, err := json.Marshal(manifest)
	if err != nil {
		return ocispec.Descriptor{}, err
	}

	return oras.TagBytes(ctx, memoryStore, ocispec.MediaTypeImageManifest,
		manifestData, parsedRef.String())
}
Repository: helm/helm
File: pkg/lint/rules/crds.go (Go)

Code before:
)

// Crds lints the CRDs in the Linter.
func Crds(linter *support.Linter) {
	fpath := "crds/"
	crdsPath := filepath.Join(linter.ChartDir, fpath)

	// crds directory is optional
	if _, err := os.Stat(crdsPath); errors.Is(err, fs.ErrNotExist) {
		return
	}

	linter.RunLinterRule(support.WarningSev, fpath, validateCrdsDir(crdsPath))

	// Load chart and parse CRDs
	chart, err := loader.Load(linter.ChartDir)

	chartLoaded := linter.RunLinterRule(support.ErrorSev, fpath, err)

	if !chartLoaded {
		return
	}

	/* Iterate over all the CRDs to check:
	- It is a YAML file and not a template
	- The kind is CustomResourceDefinition
	*/
	for _, crd := range chart.CRDObjects() {
		fileName := crd.Name
		fpath = fileName

		decoder := yaml.NewYAMLOrJSONDecoder(bytes.NewReader(crd.File.Data), 4096)
		for {
			var yamlStruct *K8sYamlStruct

			err := decoder.Decode(&yamlStruct)
			if err == io.EOF {
				break
			}

			// If YAML parsing fails here, it will always fail in the next block as well, so we should return here.
			// This also confirms the YAML is not a template, since templates can't be decoded into a K8sYamlStruct.
			if !linter.RunLinterRule(support.ErrorSev, fpath, validateYamlContent(err)) {
				return
			}

			linter.RunLinterRule(support.ErrorSev, fpath, validateCrdKind(yamlStruct))
		}
	}
}


Review comment (suggestion):
very minor nit, a numbered list would read better here:
```suggestion
	1. It is a YAML file and not a template
	2. The kind is CustomResourceDefinition
```

Code after:
	"helm.sh/helm/v4/pkg/lint/support"
)

// Crds lints the CRDs in the Linter.
func Crds(linter *support.Linter) {
	fpath := "crds/"
	crdsPath := filepath.Join(linter.ChartDir, fpath)

	// crds directory is optional
	if _, err := os.Stat(crdsPath); errors.Is(err, fs.ErrNotExist) {
		return
	}

	crdsDirValid := linter.RunLinterRule(support.ErrorSev, fpath, validateCrdsDir(crdsPath))
	if !crdsDirValid {
		return
	}

	// Load chart and parse CRDs
	chart, err := loader.Load(linter.ChartDir)

	chartLoaded := linter.RunLinterRule(support.ErrorSev, fpath, err)

	if !chartLoaded {
		return
	}

	/* Iterate over all the CRDs to check:
	1. It is a YAML file and not a template
	2. The API version is apiextensions.k8s.io
	3. The kind is CustomResourceDefinition
	*/
	for _, crd := range chart.CRDObjects() {
		fileName := crd.Name
		fpath = fileName

		decoder := yaml.NewYAMLOrJSONDecoder(bytes.NewReader(crd.File.Data), 4096)
		for {
			var yamlStruct *k8sYamlStruct

			err := decoder.Decode(&yamlStruct)
			if err == io.EOF {
				break
			}

			// If YAML parsing fails here, it will always fail in the next block as well, so we should return here.
			// This also confirms the YAML is not a template, since templates can't be decoded into a K8sYamlStruct.
			if !linter.RunLinterRule(support.ErrorSev, fpath, validateYamlContent(err)) {
				return
			}
Repository: helm/helm
File: pkg/lint/rules/crds.go (Go)

Code before:

	linter.RunLinterRule(support.WarningSev, fpath, validateCrdsDir(crdsPath))

	// Load chart and parse CRDs
	chart, err := loader.Load(linter.ChartDir)

	chartLoaded := linter.RunLinterRule(support.ErrorSev, fpath, err)

	if !chartLoaded {
		return
	}

	/* Iterate over all the CRDs to check:
	- It is a YAML file and not a template
	- The kind is CustomResourceDefinition
	*/
	for _, crd := range chart.CRDObjects() {
		fileName := crd.Name
		fpath = fileName

		decoder := yaml.NewYAMLOrJSONDecoder(bytes.NewReader(crd.File.Data), 4096)
		for {
			var yamlStruct *K8sYamlStruct

			err := decoder.Decode(&yamlStruct)
			if err == io.EOF {
				break
			}

			// If YAML parsing fails here, it will always fail in the next block as well, so we should return here.
			// This also confirms the YAML is not a template, since templates can't be decoded into a K8sYamlStruct.
			if !linter.RunLinterRule(support.ErrorSev, fpath, validateYamlContent(err)) {
				return
			}

			linter.RunLinterRule(support.ErrorSev, fpath, validateCrdKind(yamlStruct))
		}
	}
}

// Validation functions
func validateCrdsDir(crdsPath string) error {
	fi, err := os.Stat(crdsPath)
	if err != nil {
		return err
	}
	if !fi.IsDir() {
		return errors.New("not a directory")
	}
	return nil
}

Review comment (performance):
Is there a reason you chose `NewYAMLOrJSONDecoder` rather than e.g. `NewYAMLReader`?

Using `NewYAMLOrJSONDecoder` doesn't "hurt", but given JSON is subset of YAML, we don't need the extra complexity(?)


Code after:
	}

	crdsDirValid := linter.RunLinterRule(support.ErrorSev, fpath, validateCrdsDir(crdsPath))
	if !crdsDirValid {
		return
	}

	// Load chart and parse CRDs
	chart, err := loader.Load(linter.ChartDir)

	chartLoaded := linter.RunLinterRule(support.ErrorSev, fpath, err)

	if !chartLoaded {
		return
	}

	/* Iterate over all the CRDs to check:
	1. It is a YAML file and not a template
	2. The API version is apiextensions.k8s.io
	3. The kind is CustomResourceDefinition
	*/
	for _, crd := range chart.CRDObjects() {
		fileName := crd.Name
		fpath = fileName

		decoder := yaml.NewYAMLOrJSONDecoder(bytes.NewReader(crd.File.Data), 4096)
		for {
			var yamlStruct *k8sYamlStruct

			err := decoder.Decode(&yamlStruct)
			if err == io.EOF {
				break
			}

			// If YAML parsing fails here, it will always fail in the next block as well, so we should return here.
			// This also confirms the YAML is not a template, since templates can't be decoded into a K8sYamlStruct.
			if !linter.RunLinterRule(support.ErrorSev, fpath, validateYamlContent(err)) {
				return
			}

			linter.RunLinterRule(support.ErrorSev, fpath, validateCrdAPIVersion(yamlStruct))
			linter.RunLinterRule(support.ErrorSev, fpath, validateCrdKind(yamlStruct))
		}
	}
}

// Validation functions
func validateCrdsDir(crdsPath string) error {
	fi, err := os.Stat(crdsPath)
	if err != nil {
		return err
Repository: helm/helm
File: pkg/lint/rules/crds_test.go (Go)

Code before:
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package rules

import (
	"strings"
	"testing"

	"helm.sh/helm/v4/pkg/lint/support"
)

const crdsTestBaseDir = "./testdata/withcrd"
const invalidCrdsDir = "./testdata/invalidcrdsdir"

func TestCrdsDir(t *testing.T) {
	linter := support.Linter{ChartDir: crdsTestBaseDir}
	Crds(&linter)
	res := linter.Messages

	if len(res) > 0 {
		t.Fatalf("Expected no errors, got %d, %v", len(res), res)
	}
}

func TestInvalidCrdsDir(t *testing.T) {
	linter := support.Linter{ChartDir: invalidCrdsDir}
	Crds(&linter)
	res := linter.Messages

	if len(res) != 1 {
		t.Fatalf("Expected one error, got %d, %v", len(res), res)
	}

	if !strings.Contains(res[0].Err.Error(), "not a directory") {
		t.Errorf("Unexpected error: %s", res[0])
	}
}


Review comment (suggestion):
we make heavy usage of https://github.com/stretchr/testify in Helm already, this could be written as:
```
assert.Emptyf(t, res, "Expected no errors, got %v", res)
```

Code after:
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package rules

import (
	"testing"

	"github.com/stretchr/testify/assert"

	"helm.sh/helm/v4/pkg/lint/support"
)

const invalidCrdsDir = "./testdata/invalidcrdsdir"

func TestInvalidCrdsDir(t *testing.T) {
	linter := support.Linter{ChartDir: invalidCrdsDir}
	Crds(&linter)
	res := linter.Messages

	assert.Len(t, res, 1)
	assert.ErrorContains(t, res[0].Err, "not a directory")
}