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

Add more tests for buf.work.yaml and normalize and validate DirPaths #2613

Merged
merged 7 commits into from
Nov 21, 2023

Conversation

oliversun9
Copy link
Contributor

@oliversun9 oliversun9 commented Nov 20, 2023

This PR adds tests for buf_work_yaml_file.go.

This PR also fixes a bug where newBufWorkYAMLFile does not set dirPaths as the normalized input paths (but set dirPaths as the input paths as-is).

if err := validateDirPathsNoOverlap(slicesextended.MapToSortedSlice(dirPathMap)); err != nil {
return err
// TODO: use this line:
// sortedNormalDirPaths := slicesextended.MapKeysToSortedSlice(normalDirPathToDirPath)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@bufdev bufdev left a comment

Choose a reason for hiding this comment

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

You'll also want to update the documentation for DirPaths to state that paths are normalized and validated when constructing a BufWorkYAMLFile, and that DirPaths returns only normalized and validated paths.

@@ -129,13 +129,13 @@ type bufWorkYAMLFile struct {
}

func newBufWorkYAMLFile(fileVersion FileVersion, dirPaths []string) (*bufWorkYAMLFile, error) {
if err := validateBufWorkYAMLDirPaths(dirPaths); err != nil {
sortedNormalDirPaths, err := validateBufWorkYAMLDirPaths(dirPaths)
Copy link
Member

Choose a reason for hiding this comment

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

We've used normalized throughout rest of codebase, not normal, update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in f2481c7

@oliversun9
Copy link
Contributor Author

You'll also want to update the documentation for DirPaths to state that paths are normalized and validated when constructing a BufWorkYAMLFile, and that DirPaths returns only normalized and validated paths.

fixed in 4ee426d

@bufdev bufdev changed the title Add tests for buf work yaml and make DirPaths() return normal paths Add more tests for buf.work.yaml and normalize and validate DirPaths Nov 21, 2023
@bufdev bufdev merged commit 0ffaae8 into bufmod Nov 21, 2023
4 of 8 checks passed
@bufdev bufdev deleted the osun/tests-for-buf-work-yaml branch November 21, 2023 02:59
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

Successfully merging this pull request may close these issues.

2 participants