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

[Enhancement] Add diff lib for kcl test assert errors. #112

Closed
Peefy opened this issue Dec 5, 2023 · 12 comments · Fixed by #138
Closed

[Enhancement] Add diff lib for kcl test assert errors. #112

Peefy opened this issue Dec 5, 2023 · 12 comments · Fixed by #138
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@Peefy
Copy link
Contributor

Peefy commented Dec 5, 2023

Enhancement

For example, I want to obtain diff information from the error message in the following code.

import yaml
import difflib

test_merge = lambda {
    original = {
        "metadata": {
            "name": "my-deployment",
            "labels": {
                "app": "my-app",
            },
        },
        "spec": {
            "replicas": 3,
            "template": {
                "spec": {
                    "containers": [
                        {
                            "name": "my-container-1",
                            "image": "my-image-1",
                        },
                        {
                            "name": "my-container-2",
                            "image": "my-image-2",
                        },
                    ],
                },
            },
        },
    }

    patch = {
        "metadata": {
            "labels": {
                "version": "v1",
            },
        },
        "spec": {
            "replicas": 4,
            "template": {
                "spec": {
                    "containers": [
                        {
                            "name": "my-container-1",
                            "image" = "my-new-image-1",
                        },
                        {
                            "name": "my-container-3",
                            "image" = "my-image-3",
                        },
                    ],
                },
            },
        },
    }
    expected = yaml.decode("""\
metadata:
  name: my-deployment
  labels:
    app: my-app
    version: v1
  spec:
    replicas: 4
    template:
      spec:
        containers:
        - name: my-container-1
          image: my-new-image-1
        - name: my-container-2
          image: my-image-2
        - name: my-container-3
          image: my-image-3
""")

    got = merge(original, patch)
    diff = difflib.diff(got, expected)
    assert str(got) == str(expected), "expected ${expected}, got ${got}, the diff is ${diff}"
}
@Peefy Peefy added help wanted Extra attention is needed good first issue Good for newcomers labels Dec 5, 2023
@abhayporwals
Copy link

Hey @Peefy, I would like to work on this.
Share some more details that helps, if you can.

@Peefy
Copy link
Contributor Author

Peefy commented Dec 6, 2023

Hey @Peefy, I would like to work on this. Share some more details that helps, if you can.

Hi @professorabhay. Thank you for contributing to the KCL community. The expectation is to provide a diff library written by KCL to display the differences between KCL configurations at the module repo: https://github.com/kcl-lang/modules

@Peefy
Copy link
Contributor Author

Peefy commented Dec 19, 2023

Hello @professorabhay If you have any difficulties, please feel free to communicate at any time. According to that the issue has been open for over two weeks.

@abhayporwals
Copy link

Hey @Peefy, I have my exams ongoing but will try to resolve it and raise a PR by the end of Thursday.
Sorry for the inconvenience.

@abhayporwals
Copy link

Hey @Peefy, First of all, sorry for the delay. I give it a try but having some hurdles while resolving it.
I couldn't find any diff library written by KCL.
Please provide more context about it.

@Peefy Peefy transferred this issue from kcl-lang/kcl Jan 11, 2024
@Peefy
Copy link
Contributor Author

Peefy commented Jan 11, 2024

Hi @professorabhay, I'm glad you're still working here. The purpose of this issue is to use KCL to develop a difflib library for string diff, just like how Python's difflib does. The things that need to be done are as follows:

  • Create a new module called difflib in the kcl-lang/modules repository using kcl v0.7+ and submit the PR to here
  • Implementing a simple difflib library using KCL can display the diff of string arrays, just like:
import difflib

print("\n".join(difflib.ndiff("a\na".splitlines(), "a\nb".splitlines())))

The output is

  a
- a
+ b

@Peefy Peefy changed the title [Enhancement] Add diff feature for the assert errors. [Enhancement] Add diff lib for kcl assert errors. Jan 11, 2024
@Peefy Peefy changed the title [Enhancement] Add diff lib for kcl assert errors. [Enhancement] Add diff lib for kcl test assert errors. Jan 11, 2024
@abhayporwals
Copy link

Ok, Thanks for the update @Peefy.
Take a look at the structure - [KCL v0.7+]

image

@Peefy
Copy link
Contributor Author

Peefy commented Jan 11, 2024

Ok, Thanks for the update @Peefy.
Take a look at the structure - [KCL v0.7+]

image

Good Job

@abhayporwals
Copy link

What to write in this main.k file or should I raise the PR?

@Peefy
Copy link
Contributor Author

Peefy commented Jan 13, 2024

What to write in this main.k file or should I raise the PR?

You can raise the PR. The next step is to write the implementation of difflib in main.k, and you can refer to the implementation of difflib in Python

@abhayporwals
Copy link

Please take a look to that -

class DiffLib {
    companion object {
        fun diff(seq1: CharSequence, seq2: CharSequence): List<String> {
            val result = mutableListOf<String>()

            val dp = Array(seq1.length + 1) { IntArray(seq2.length + 1) }

            for (i in 0..seq1.length) {
                for (j in 0..seq2.length) {
                    when {
                        i == 0 -> dp[i][j] = j
                        j == 0 -> dp[i][j] = i
                        seq1[i - 1] == seq2[j - 1] -> dp[i][j] = dp[i - 1][j - 1]
                        else -> dp[i][j] = 1 + minOf(dp[i - 1][j], dp[i][j - 1], dp[i - 1][j - 1])
                    }
                }
            }

            var i = seq1.length
            var j = seq2.length

            while (i > 0 || j > 0) {
                when {
                    i > 0 && j > 0 && seq1[i - 1] == seq2[j - 1] -> {
                        result.add("  ${seq1[i - 1]}")
                        i--
                        j--
                    }
                    j > 0 && (i == 0 || dp[i][j - 1] <= dp[i - 1][j]) -> {
                        result.add("+ ${seq2[j - 1]}")
                        j--
                    }
                    i > 0 && (j == 0 || dp[i][j - 1] > dp[i - 1][j]) -> {
                        result.add("- ${seq1[i - 1]}")
                        i--
                    }
                }
            }


            return result.reversed()
        }
    }
}

@Peefy
Copy link
Contributor Author

Peefy commented Jan 13, 2024

The logic is right. You can translate it to KCL and raise a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants