[ST-NNNN] Tag-based test execution filtering

[gmedori]

For convenience, here's a link to the implementation of the feature described in this proposal: swiftlang/swift-testing#1531

[grynspan]

Should we just make it a policy that \ escapes the next character? If not, consider adding some discussion in this area to Alternatives Considered.

Or, if that's impractical or undesirable, should we document how to write \: verbatim (i.e. we probably also need to allow escaping \ itself)?

[gmedori]

Personally I think that if we want to make that a policy, the policy should exist at SPM level for every command. I'm not sure it's useful to have a policy that only applies to swift testing. Probably skews impractical IMO. Think this is something we want to pursue anyway?

Or, if that's impractical or undesirable, should we document how to write : verbatim (i.e. we probably also need to allow escaping \ itself)?

Do you mean in the usage description of the --skip and --filter options?

[grynspan]

This discussion might belong in Source Compatibility rather than this section because it doesn't really affect e.g. VS Code or Xcode integrations.

[gmedori]

Yeah I read through that last night and think I agree.

[gmedori]

I moved this discussion to Source Compatibility and added some clearer thoughts around tool integrations in 9005487

[allevato]

You should also consider what needs to happen (if anything different) if the tag itself is a raw identifier (unless anything else in Swift Testing prohibits that, which I don't know off the top of my head):

@Test(.tags(.`some tag with spaces`))
func `my test`() { ... }

I think I would expect to write swift test --skip 'tag:some tag with spaces' here.

[grynspan]

Then there's an open question about whether or not developers should specify backticks (which also affect shell processing).

[gmedori]

ha, yep that complicates things. I think the most correct answer here is to write it as @allevato put it. Involving backticks opens the command up to different interpretations in different shells, where as single-quoting is relatively universal, even going back to shells like sh and csh.

[gmedori]

Added some more discussion around this scenario in 9005487

[allevato]

As a parallel, when using raw identifiers in the -module-alias or -module-abi-name command line flags of the compiler, you don't need to include the backticks: you'd write -module-abi-name 'foo bar', not -module-abi-name '`foo bar`'. So I think it's reasonable to have the same policy here.

In other words, the backticks are required to delimit the identifier in source code (and they also appear in the mangling of the symbol for compatibility reasons), but it's reasonable to drop them elsewhere.

[grynspan]

I don't have an opinion either way, but we should be clear about it. (And if the developer does type the backticks, does that make it wrong?)

[gmedori]

if the developer does type the backticks, does that make it wrong?

I could go either way on this, so IMO we should just fall back to precedent. From what @allevato said:

you'd write -module-abi-name 'foo bar', not -module-abi-name '`foo bar`'

it sounds like using backticks would be incorrect. So I'd opt to not do any special handling around stripping them for the user. But please correct me if I'm misinterpreting.

[allevato]

If you include the backticks in -module-abi-name, it looks like it checks whether the symbol needs to be escaped (which it does, because it contains a space) and then inserts a second pair of backticks, so the mangled symbol ends up being ``foo bar``. This is actually wrong because raw identifiers are not supposed to be able to contain backticks (syntactically, at least). The frontend should prevent that instead of letting it through.

So maybe that flag wasn't the best one to cite, but my vote here would be "don't require backticks and fail if the user provides them".

[gmedori]

I think this might be a common enough occurrence that there's actually value in detecting whether backticks were supplied and presenting a targeted error message. I'll work on adding that to the implementation later today.