homeport / dyff

/ˈdʏf/ - diff tool for YAML files, and sometimes JSON
MIT License
1.27k stars 60 forks source link

Support comparing files with different numbers of documents #16

Open HeavyWombat opened 6 years ago

HeavyWombat commented 6 years ago

Have a look at YAML files to compare where each YAML file contains a different number of documents in it. This might include a change in the way paths are displayed.

Tommyf commented 1 year ago

I just stumbled on this problem. I am using dyff to compare helm template output against some predefined expected output.

This all works fine when the same number of K8s manifest documents are in the yaml output, but it falls flat if making changes that add/remove manifests.

Is there any progress being made on this issue? If not, would it be OK for me to have a crack at it? I don't want to double up on any efforts.

Tommyf commented 1 year ago

Hooo boy. I just wasted a day on this, trying to implement the logic to do this. I got quite far down the rabbit hole before realizing it already does this for source files that are detected to be Kubernetes resources. I was confused, because the code suggested that, with K8s manifest YAML files, I should never reach the code block that errors if the number of documents are unequal.

It was just a few minutes ago that I discovered the reason for my confusion. My helm template command was failing and not outputting valid YAML, so the K8s auto-detection did not succeed and it followed through to the conventional logic, which fails on mismatched doc quantities.

I'm a fool.

I found the true cause, as I was writing test cases for this scenario in the dyff code base, but they succeeded instead of failed. Yay for TDD! I may still submit my code changes as it's always nice to have more tests for clarity.

HeavyWombat commented 1 year ago

Hey, sorry for the late reply. So, yes, there is support for comparing multiple documents ... so technically speaking this shows that I am not very good in keeping the issues up to date.

Like you said, the compare only works for Kubernetes resources currently, because these can be identified by their Kind and Name. So far, I have not received feedback for other YAMLs where the same would be the case. At first, this behaviour needed to be explicitly enabled, but with some release I changed that. Mostly, for convenience as Kubernetes YAMLs are the ones I work the most with.

If there is anything that can be improve with the handling, or importantly, the error messages, I am happy to have a look at it.

rbtcollins commented 5 months ago

I believe there is an unhandled edge case.

I wanted to use dyff under git using this wrapper:

#!/bin/sh
# path old-file old-hex old-mode new-file new-hex new-mode
# for deleted /new files, use /dev/null instead
old="$2"
new="$1"

if [ ! -f "$old" ]; then 
    old=/dev/null
fi
if [ ! -f "$new" ]; then 
    new=/dev/null
fi

echo $1 $new 
echo $2 $old
dyff between "$old" "$new" 

But this failed like so:

path-in-repo-do-deleted-foo.yaml /dev/null
/tmp/git-blob-FOWoJY/foo.yaml /tmp/git-blob-FOWoJY/foo.yaml
╭ Error occurred
│ failed to compare input files: comparing YAMLs with a different number of
│ documents is currently not supported
╵
fatal: external diff died, stopping at path-in-repo-do-deleted-foo.yaml

so the edge case is an entirely empty file doesn't detect as Kubernetes. But it should right? because every element of the list that makes up the multi-document YAML stream is a valid Kubernetes resource.

MatrixManAtYrService commented 2 months ago

I can reproduce this error in 1.8.0 by comparing an empty file with one that has two documents:

$ touch old.yaml
$ dyff between old.yaml new.yaml
╭ Error occurred
│ failed to compare input files: comparing YAMLs with a different number of documents is
│ currently not supported
╵

My workaround is to use a file with contents --- instead of an empty one:

$ rm old.yaml
$ '---' | save old.yaml
$ dyff between old.yaml new.yaml
     _        __  __
   _| |_   _ / _|/ _|  between old.yaml
 / _' | | | | |_| |_       and new.yaml, two documents
| (_| | |_| |  _|  _|
 \__,_|\__, |_| |_|   returned one difference
        |___/

(file level)
    ---
    apiVersion: v1
    kind: Namespace
    metadata:
    │ name: foo
    apiVersion: v1
    kind: ServiceAccount
    metadata:
    │ name: bar
    │ namespace: foo