Skip to content

Conversation

@rudfoss-rr
Copy link

@rudfoss-rr rudfoss-rr commented Nov 15, 2024

Checklist

  • Tests added / updated
  • Docs added / updated

Does this PR introduce a breaking change?

  • Yes
  • No

If indicated yes above, please describe the breaking change(s).

Remove this quote before creating the PR.

Additional context

Suggestion for adding support for a parser argument to the CLI so that the appropriate parser can be selected for Json files.

@rudfoss-rr rudfoss-rr requested a review from a team as a code owner November 15, 2024 10:14
@rudfoss-rr
Copy link
Author

@mnaumanali94 I was having trouble fixing the commit message issues so I reopened a new PR instead. Sorry for the confusion. I've addressed the lint issues, but I'm having trouble getting all tests to work as well as adding appropriate tests of my own. Would love some help if you have time as I haven't worked with test-harness before and can't seem to get them to run locally in dev-container.

@rudfoss-rr rudfoss-rr mentioned this pull request Nov 15, 2024
4 tasks
@mnaumanali94
Copy link
Contributor

@rudfoss-rr So to run harness tests you first need to build the binaries. For that cd into the package/cli folder and run yarn build.binary and then at the root level run yarn test.harness. Let us know if you run into issues!

@rudfoss-rr
Copy link
Author

@rudfoss-rr So to run harness tests you first need to build the binaries. For that cd into the package/cli folder and run yarn build.binary and then at the root level run yarn test.harness. Let us know if you run into issues!

Thanks! I've updated tests and attempted to add a few scenarios of my own, but was unable to completely rebuild the scenario tests using either test.harness, build or build.binary so I wasn't able to properly verify all scenarios locally. I'm guessing there is just some script I'm missing.

I now see that I've broken a few other tests, but I'm not sure exactly why. If you could have a look and point me in the right direction that would be great!

@rudfoss-rr rudfoss-rr closed this by deleting the head repository Jul 17, 2025
@sbstnkr
Copy link

sbstnkr commented Aug 11, 2025

@rudfoss-rr this hasn't been addressed, correct? I still see the warnings for \t in json files

@rudfoss-rr
Copy link
Author

@sbstnkr Yes, that is right. But I was unable to get the tests working and had to move on to other work so I closed it to avoid polluting the project with a broken PR.

@sbstnkr
Copy link

sbstnkr commented Aug 18, 2025

@rudfoss-rr could you please create another PR? 🙏🏼

@rudfoss-rr rudfoss-rr mentioned this pull request Aug 19, 2025
4 tasks
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.

3 participants