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

yamlfmt -dry and yamlfmt (no -dry) have a difference of opinion #155

Open
chris-braidwell opened this issue Jan 30, 2024 · 1 comment
Open

Comments

@chris-braidwell
Copy link

chris-braidwell commented Jan 30, 2024

SUMMARY

yamlfmt in fix mode will change files that yamlfmt -dry says don't need to be changed.

The changes made by yamlfmt are then noted as formatting failures by yamlfmt -lint.

Each time I run yamlfmt, it adds more newlines to the same file -- is not idempotent.

DETAILS

Running fix mode / dry run does not find files to change. But running fix mode / live makes a change. Running lint mode complains a file that was just changed by fix mode / live needs to change again. I manually check the file and it does have blank lines that need to be removed. Thus fix mode / live seems to disagree with lint mode or fix mode / dry run:

❯ git status | grep yml

❯ yamlfmt -conf ./.yamlfmt -dry
2024/01/30 17:08:04 No files will be changed.

❯ yamlfmt -conf ./.yamlfmt

❯ git status | grep yml
	modified:   path/to/some/yaml/file.yml

❯ yamlfmt -conf ./.yamlfmt -lint -quiet
2024/01/30 17:18:13 The following files had formatting differences:

path/to/some/yaml/file.yml

Also, if I run fix mode / live on the same file repeatedly, it grows every time (as yamlfmt adds 4 newlines per run):

❯ wc path/to/some/yaml/file.yml
 215  781 6800 path/to/some/yaml/file.yml

❯ yamlfmt -conf ./.yamlfmt

❯ wc path/to/some/yaml/file.yml
 219  781 6804 path/to/some/yaml/file.yml

❯ yamlfmt -conf ./.yamlfmt

❯ wc path/to/some/yaml/file.yml
 223  781 6808 path/to/some/yaml/file.yml

I'm doing this in a dev-local environment where the .yml/.yaml files are not being changed by anything but yamlfmt.

Observations:

  • It seems yamlfmt fix mode isn't fully reliable.
  • If yamlfmt -dry reports no changes and yamlfmt then makes changes, that is a defect. Maybe there should be tests that run yamlfmt -dry first - if it reports no changes needed then yamlfmt does make changes, that is a failure case.
  • If yamlfmt makes changes and yamlfmt -lint fails on a just-changed file, that is a defect. Maybe there should be tests that run yamlfmt - note changed files - if yamlfmt -lint doesn't accept those files as-is, that is a failure case.
  • If yamlfmt makes changes, then is run again and makes changes to the same file (like adding newlines), that is a defect. Maybe there should be tests that run yamlfmt twice - if any file changes in both runs, that is a failure case.
  • This is failing in just one .yml file in a repo that has many .yml files - nothing special about that file.
  • Every time I run yamlfmt, it adds one more blank line to that file (observed using git diff).

ENVIRONMENT

I'm running yamlfmt version 0.10.0.

This happens with a completely empty config file.

WHAT'S NEXT

I can provide a copy of the problem .yml file if that'd help.

@braydonk
Copy link
Collaborator

braydonk commented Jan 31, 2024

Hi @chris-braidwell thanks for opening an issue.

I will definitely need a copy of the yaml that caused this issue to occur. I am not able to reproduce this always adding a newline behaviour on any yaml I have. It is probably a bug in the yaml library.

If the yaml causes there to always be a newline added, then the behaviour of lint is working as intended; lint is designed to output a negative result if any file would have been changed, and even though it's an unintentional affect, under the scenario you provided there always would be a file changed according to lint. This probably does expose a bug in how dry is calculated, as I would expect that to have the same result as lint always detecting this one file will be changed.

All the test cases you suggested would be good to add to integrationtest.

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

2 participants