Fix --quiet option not working with swift run #8844

[MaelRB]

Fixes an issue where the --quiet option had no effect when used with swift run and swift build command.

Motivation:

When using the --quiet option with swift run only error messages should be logged.

Modifications:

• Add isQuiet property to filter out messages based on the log level during build phases of the native, xcode and swiftbuild build system.
• Remove duplicated declaration of the isVerbose property from different targets.

Result:

When the --quiet option is used with the swift run and swift build command only error messages are logged.

[plemarquand]

Looks good, thanks for this! A few little nits, but nothing major.

[plemarquand]

nit: roll this up into the guard above

[plemarquand]

nit: just for symmetry with isVerbose

Suggested change

	self == .error
	self >= .error

[plemarquand]

@swift-ci please test

[bkhouri]

Thanks for this. However, to ensure we do not regress, could I ask that you add a test for the for each SwiftPM executable, where appropriate?

Take a look in Tests/CommandsTests/*CommandTests.swift.

[MaelRB]

Thanks for this. However, to ensure we do not regress, could I ask that you add a test for the for each SwiftPM executable, where appropriate?

Take a look in Tests/CommandsTests/*CommandTests.swift.

Yes, sure! Thanks for the hint. I think I have an idea on how to do that properly.

[MaelRB]

@bkhouri I don't know how we should use fixtures folders. Should we have a unique folder per test or is it ok to reuse them ? Since test suites are serialized I went for the second option, there shouldn't be any concurrent access to fixtures folders causing flaky tests.

[bkhouri]

@bkhouri I don't know how we should use fixtures folders. Should we have a unique folder per test or is it ok to reuse them ? Since test suites are serialized I went for the second option, there shouldn't be any concurrent access to fixtures folders causing flaky tests.

@MaelRB : You can re-use fixtures were necessary :). When using the fixture(name: ...) function, it will copy the fixture to a temporary directory, so there is no "overlap".

[MaelRB]

I've noticed an inconsistency on which output stream is used by a build system to raise the error when the build failed with the build command. SwiftBuild uses stderr but native and xcode use stdout. Is it the wanted behavior ?

From the swiftBuildQuietLogLevelWithError method:

if buildSystem == .swiftbuild {
   // THEN we should see output in stderr
   #expect(stderr.isEmpty == false)
   // AND no content in stdout
   #expect(stdout.isEmpty)
} else {
   // THEN we should see content in stdout
   #expect(stdout.isEmpty == false)
   // AND no output in stderr
   #expect(stderr.isEmpty)
}

The run command is consistent though

[plemarquand]

@swift-ci please test

[bkhouri]

I've noticed an inconsistency on which output stream is used by a build system to raise the error when the build failed with the build command. SwiftBuild uses stderr but native and xcode use stdout. Is it the wanted behavior ?

@MaelRB Please file an issue and we can looking into it :). Feel free to reference the test (and PR).

[bkhouri]

Thanks you for adding tests and working through the comments. it's really appreciated

[bkhouri]

@swift-ci test

[bkhouri]

@MaelRB: You may need to rebase (or merge main) into your branch to pick the change that address the build failures.

[plemarquand]

@swift-ci please test

[MaelRB]

@bkhouri @plemarquand Should I wrap test cases within a withKnownIssue block to pass the CI ? Most of the tests in RunCommandTests and BuildCommandTests do that

[bkhouri]

@bkhouri @plemarquand Should I wrap test cases within a withKnownIssue block to pass the CI ? Most of the tests in RunCommandTests and BuildCommandTests do that

@MaelRB : if they are related to SwiftBuild, I would say you can mark them withKnownIssue, and try to include a .bug(...) trait on the test referring to the change that "should" address it.

[MaelRB]

@bkhouri @plemarquand Should I wrap test cases within a withKnownIssue block to pass the CI ? Most of the tests in RunCommandTests and BuildCommandTests do that

@MaelRB : if they are related to SwiftBuild, I would say you can mark them withKnownIssue, and try to include a .bug(...) trait on the test referring to the change that "should" address it.

I marked two of the tests withKnownIssue. I couldn't link them to any known .bug(...) though.