On Tue, Aug 6, 2024 at 6:10 AM AbdAlRahman Gad <abdobngad@xxxxxxxxx> wrote: > On 8/6/24 06:02, Eric Sunshine wrote: > > We need to exercise a bit of caution when making this sort of change > > because a "..." string differs from a '...' string in shell. [...] > > [...] > > Having said all that, in this case, you seem to have lucked out and > > nothing broke by changing the double-quotes to single-quotes around > > the test bodies. > > > > However... > > ... changing the double-quotes to single-quotes for the test _titles_ > > in these instances is actively wrong. In this case, we _want_ > > interpolation of `$option` to happen in the title string so that the > > output looks like this: > > ok 167 - mixing incompatible modes with --contains is forbidden > > > > By changing the title to use single-quotes, you suppress interpolation > > of `$option`, with the result that the displayed titles become rather > > useless: > > ok 167 - mixing incompatible modes with $option is forbidden > > Thanks for the through explanation! Should I not include this patch in > v5 as the test body is error-prune or should I revert changing the > double-quotes to single-quotes for test description and keep the changes > to the test body and send the patch with v5? Overall, the changes made by this patch are valuable since it brings the script more in line with modern practice and because single-quotes around the test body make it easier for readers to reason about test behavior. Coupled with the fact that none of the tests broke by changing the bodies from double- to single-quote, it's a good idea to keep this patch in v5 but omit the parts which break the test descriptions.
