Adapt replacements to modify json schema tests

[miriamgreis]

When using Portman, we wanted to use replacements to modify the generated JSON schema tests. However, we couldn't get it to work as described in the documentation.

To test our use cases, I added two tests which either replace parts of the JSON schema with an empty string (to delete parts) or another string (to modify parts). I then corrected the used pattern in the writeRawReplacements function to make the tests pass.

[thim81]

@miriamgreis Thanks for the contribution. The change to the "writeRawReplacements" is rather straightforward.

Could you explain your primary use-case a bit more in detail?
What do you try to achieve? What was not working, based on the documentation?

[thim81]

Could you have a look at the "lint" validation step, since it complaints about the improper escaped values?
https://github.com/apideck-libraries/portman/runs/6835570728?check_suite_focus=true

[thim81]

@nicklloyd Could you review the change and the impact?

[miriamgreis]

@miriamgreis Thanks for the contribution. The change to the "writeRawReplacements" is rather straightforward.

Could you explain your primary use-case a bit more in detail? What do you try to achieve? What was not working, based on the documentation?

What we want to do: we want to remove parts of the JSON schema (as the format: date in the test I wrote) because we have legacy API definition that use this "format: date", but the API doesn't provide dates in the correct format. We cannot just adapt the definitions right now, but we want to run some tests. So for us the easiest way to get the tests to run would be using replacements.

The documentation states "By defining portmanReplacements, you can modify any snippet that is injected by Portman, like the test names, correct part of the JSON schema, ... that you would not be able to modify by the available "overwrite" methods." Unfortunately, the documentation doesn't provide any example of how to do this. After trying with different searchString without getting it to work, we started digging in the code and noticed that the "searchFor.replace(/"/g, '\\"')" might have a problem, so the easiest way for me was writing a test how I expect it to work and adapt the code.

I'll take a look at the lint step and code scanning results as soon as possible.

[thim81]

@miriamgreis Thanks for explaining the use-case. The usage of writeRawReplacements would indeed solve your need.
Of course be cautious since it is quite a "raw" replacement.

If you could fix the linting error in the coming days, we could include it the upcoming release.

[miriamgreis]

@thim81 I had to add prettier-ignore and eslint-disable to fix the linting errors. The escaped characters are necessary for the test because that's exactly how the generated output will look like. I hope that's ok.

[thim81]

@miriamgreis It looks good, lets see what the tests say.
The timing is perfect, since we could include it in the upcoming release of Portman.

[thim81]

@miriamgreis The code scanning steps warns about the security risk

Incomplete string escaping or encoding - High
This does not escape backslash characters in the input.

I'm not regex expert, but perhaps you are?

[miriamgreis]

@thim81 Unfortunately not. But I'll take a closer look this afternoon.

[miriamgreis]

So, I took a closer look with one of my colleagues and improved the readability of the tests by making the examples shorter. We also realised that disabling the linter was not necessary, so I was able to remove that again.

@thim81 For the code scanner warning: we do think that it is irrelevant. It is a warning related to sanitise untrusted input to avoid injections, which doesn't have anything to do with the functionality implemented here. The exact same replacement is used in the next line of code as well. Is there an option to ignore/disable the code scanning for the line?

[nicklloyd]

IIRC, the extra escaping was there to silence the security warning...

[thim81]

@miriamgreis Thanks for taking the time to create the PR and add the tests.

We merged the PR, so it will be part of the next release that we are actively prepping.

[thim81]

@miriamgreis We just released Portman v1.18.0, which includes your PR.

Thank you for your contribution.

[miriamgreis]

Thanks! 👍

[proDOOMman]

Does it works? I still can't change '"mode": "file"' in collection json to something else

[miriamgreis]

Yes, the use case that I described works now.