solidity
solidity copied to clipboard
Pre-release warning stripping in CLI tests does not always produce correct JSON formatting when combined with `--pretty-print`
This came up in https://github.com/ethereum/solidity/pull/13162#pullrequestreview-1101249411.
Non-release builds of the compiler always output an extra warning, which results in a different output between release and debug builds in CI:
Warning: This is a pre-release compiler version, please do not use it in production.
For this reason cmdlineTests.sh script strips the warning from compile output. For Standard JSON this is more tricky, since the error is a part of an array and we have to match the JSON formatting of the expectations. The stripping code seems to have a small bug because in some cases the formatting it produces after stripping the warning differs from what we get when the warning is not there at all.
We need to adjust the stripping code to handle this corner case. It would be best if we could also solve the problem more generally by making the CLI tests simply ignore the JSON formatting.
Fixing this is likely a prerequisite for #7665, since enabling pretty printing may uncover this problem in other existing tests.
How to reproduce
Let's look at the prettified Standard JSON output for this input.json file:
{
"language": "Solidity",
"sources": {
"url_not_found.sol": {
"urls": ["contract.sol"]
}
}
}
solc --standard-json input.json --pretty-json --json-indent 4
This is what the output of a pre-release binary looks like unstripped:
{
"errors":
[
{
"component": "general",
"formattedMessage": "Cannot import url (\"contract.sol\"): File not found. Searched the following locations: \"\".",
"message": "Cannot import url (\"contract.sol\"): File not found. Searched the following locations: \"\".",
"severity": "error",
"type": "IOError"
},
{
"component": "general",
"errorCode": "3805",
"formattedMessage": "Warning: This is a pre-release compiler version, please do not use it in production.\n\n",
"message": "This is a pre-release compiler version, please do not use it in production.",
"severity": "warning",
"type": "Warning"
}
],
"sources": {}
}
The stripping in cmdlineTests.sh removes the pre-release warning, which results in this output:
{
"errors":
[
{
"component": "general",
"formattedMessage": "Cannot import url (\"contract.sol\"): File not found. Searched the following locations: \"\".",
"message": "Cannot import url (\"contract.sol\"): File not found. Searched the following locations: \"\".",
"severity": "error",
"type": "IOError"
}],
"sources": {}
}
Note the }], sequence in the output. A release binary, that does not output the warning produces a slightly different formatting in that spot:
{
"errors":
[
{
"component": "general",
"formattedMessage": "Cannot import url (\"contract.sol\"): File not found. Searched the following locations: \"\".",
"message": "Cannot import url (\"contract.sol\"): File not found. Searched the following locations: \"\".",
"severity": "error",
"type": "IOError"
}
],
"sources": {}
}
As a result, it's impossible to write a test with nicely formatted JSON output that matches both release and pre-release build output. For example in #13162 t_ubu_release fails if we expect }], and t_ubu_cli fails if we expect the other variant.
The warning stripping code
You can find the code responsible for stripping this warning here: https://github.com/ethereum/solidity/blob/1fbee8259aca10ffe04efa47a21f31d7dee1baa9/test/cmdlineTests.sh#L189-L199