🐛 Bug Report: Repo Tools - API Reports - Process swallowed if args are not correct

[awanlin]

📜 Description

If you have a space in the command like this, the space is in the args being sent to the -o flag:

backstage-repo-tools api-reports -o ae-wrong-input-file-type, ae-undocumented --validate-release-tags

The command will only output:

# Compiling TypeScript

👍 Expected behavior

I "think" the behaviour in this case should be an error that the command is wrong and that perhaps saying it got unexpected arguments.

👎 Actual Behavior with Screenshots

If you have a space in the command like this, the space is in the args being sent to the -o flag:

backstage-repo-tools api-reports -o ae-wrong-input-file-type, ae-undocumented --validate-release-tags

The command will only output:

# Compiling TypeScript

👟 Reproduction steps

This works best when you have changes that would need the API Reports to be updated.
In this repo or the Community Plugins repo update a build:api-reports:only script to pass args to the -o flag with a space between them like this:

backstage-repo-tools api-reports -o ae-wrong-input-file-type, ae-undocumented --validate-release-tags

Then run yarn build:api-reports notice that it does not generate a new API Report

Notice: the API Reports are not updated/generated

📃 Provide the context for the Bug.

I made the mistake of adding a space when I was upgrading the Azure DevOps plugins to 1.32.x. As this does not cause any errors it was merged in. Then in follow up PRs where I expected the API Reports to change I noticed this issue and fixed it.

This is problematic as it won't get picked up by the CI and you can easily move forward without these reported being updated.

🖥️ Your Environment

This repo or the Community Plugins repo

👀 Have you spent some time to check if this bug has been raised before?

• I checked and didn't find similar issue

🏢 Have you read the Code of Conduct?

• I have read the Code of Conduct

Are you willing to submit PR?

No, but I'm happy to collaborate on a PR with someone else

[Rugvip]

Hmm, yep I think we want to error if a filtered out path is not found, i.e. ae-undocumented in this case

[jescalada]

@awanlin Oddly, I tried reproducing the issue by executing the script directly (and logging what the API reports script is getting after parsing the omitMessages argument and it seems to error out as expected:

However, when setting the build:api-reports:only script as you mentioned, the bug occurs:

The script fails to obtain the second argument. I guess we could check that the omitMessages are all valid options, or perhaps just error out if any of the options are empty strings? Let me know your thoughts about it! 🙂

I can make a PR and also update the unit tests with the extra edge case.

[jescalada]

I just made a quick PR for this issue! Given the way yarn parses the arguments by default, when adding a comma followed by a space and without wrapping with quotes, the raw omitMessages argument ends up as something that ends with a comma.

Also updated the unit tests to cover this edge case!