Skip to content

Conversation

@tothszabi
Copy link
Contributor

This PR adds better command error handling to all of the command runnner functions. This means that instead of just writing

exit status 65

to the output it will add the command details to the error like

command failed with exit status 65 (xcodebuild "-project" "/Users/szabi/Dev/misc/FlakyTests/FlakyTests.xcodeproj" "-scheme" "FlakyTests1" "test" "-destination" "id=D0D80222-0FE8-49A3-B9D6-ACAD161DF0CF" "-resultBundlePath" "/var/folders/wz/k4w2zd415xj8hv96v41jvzj00000gq/T/XCUITestOutput2241260735/Test-FlakyTests1.xcresult" "-xcconfig" "/var/folders/wz/k4w2zd415xj8hv96v41jvzj00000gq/T/1365519918/temp.xcconfig"):

It also supports custom error finding logic. This means it feeds the stderr and stdout outputs through this error finder which can return additional error lines. For example, with this we can easily fetch additional xcodebuild errors and add them as failure reasons. Here is a real life example

Run:
  command failed with exit status 65 (xcodebuild "-project" "/Users/szabi/Dev/misc/FlakyTests/FlakyTests.xcodeproj" "-scheme" "FlakyTests1" "test" "-destination" "id=D0D80222-0FE8-49A3-B9D6-ACAD161DF0CF" "-resultBundlePath" "/var/folders/wz/k4w2zd415xj8hv96v41jvzj00000gq/T/XCUITestOutput2241260735/Test-FlakyTests1.xcresult" "-xcconfig" "/var/folders/wz/k4w2zd415xj8hv96v41jvzj00000gq/T/1365519918/temp.xcconfig"):
    xcodebuild: error: The project named "FlakyTests" does not contain a scheme named "FlakyTests1". The "-list" option can be used to find the names of the schemes in the project.

}

func (c command) wrappedOutputBuffers() (*bytes.Buffer, *bytes.Buffer) {
var outBuffer, errBuffer bytes.Buffer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way, we capture the full combined output of the commands, which can take relatively big memory in case of some commands (like xcodebuild).

What about creating a custom Writer (for example, ErrorFinderWriter), which is constructed with the ErrorFinder function and runs when Write is called?
This way we don't need to hold the whole command output in memory and can be similar set for the cmd.Stdout and cmd.Stderr to the current implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only outstanding question regarding this solution was that what is happening with multiline errors. Do they delivered in one go or are they going to be split up. Well I tried it and it seems like they are delivered in one chunk and this can the right solution.

I have a sample run where I referenced non existing simulators and this multiline error was a single chunk:

error collector | Received output: xcodebuild: error: Unable to find a destination matching the provided destination specifier:
                { id:EF348E33-8191-4224-8DB6-85F27F701000 }

        The requested device could not be found because no available devices matched the request.

        Available destinations for the "FlakyTests" scheme:
                { platform:macOS, arch:arm64, variant:Designed for [iPad,iPhone], id:00008103-000C614614BB001E }
                { platform:iOS, id:dvtdevice-DVTiPhonePlaceholder-iphoneos:placeholder, name:Any iOS Device }
                { platform:iOS Simulator, id:dvtdevice-DVTiOSDeviceSimulatorPlaceholder-iphonesimulator:placeholder, name:Any iOS Simulator Device }
                { platform:iOS Simulator, id:E2A5A03C-E1EB-4FF2-9D77-56ADD545D094, OS:16.1, name:iPad (10th generation) }
                { platform:iOS Simulator, id:92E16292-4848-4A86-8151-39EE589CCD5E, OS:16.1, name:iPad Air (5th generation) }
                { platform:iOS Simulator, id:3698CDAB-A354-4AE8-8D0B-1A6DA49B05E0, OS:16.1, name:iPad Pro (11-inch) (4th generation) }
                { platform:iOS Simulator, id:F85507B3-0C4B-4103-9745-4DEE678C786B, OS:16.1, name:iPad Pro (12.9-inch) (2nd generation) }
                { platform:iOS Simulator, id:8F2138A2-40BF-4CAE-8AA9-CCAE5B4BBFD1, OS:16.1, name:iPad Pro (12.9-inch) (6th generation) }
                { platform:iOS Simulator, id:52C04C69-0F4E-486C-BC43-ADAD4DB73C35, OS:16.1, name:iPad mini (6th generation) }
                { platform:iOS Simulator, id:D0D80222-0FE8-49A3-B9D6-ACAD161DF0CF, OS:16.1, name:iPhone 8 Plus }
                { platform:iOS Simulator, id:AB22472D-1951-4F3B-8DBD-3905533528DA, OS:16.1, name:iPhone 11 Pro Max }
                { platform:iOS Simulator, id:8D2FE8B8-DA74-4ED4-B6C4-181340DB9991, OS:16.1, name:iPhone 14 }
                { platform:iOS Simulator, id:8A2787A8-B6B7-49DF-ABE2-CD2C6F5C78AA, OS:16.1, name:iPhone 14 Plus }
                { platform:iOS Simulator, id:E2A7D25B-9AB5-4279-AB5C-FFA6DFBE99AA, OS:16.1, name:iPhone 14 Pro }
                { platform:iOS Simulator, id:A4F566D1-D7EB-4416-9A78-747D48ABE498, OS:16.1, name:iPhone 14 Pro Max }
                { platform:iOS Simulator, id:B5ED6FAF-C16D-4EBC-875E-10761B76B88D, OS:16.1, name:iPhone SE (3rd generation) }

So I have updated the PR with a new error collector struct which implements the io.Writer interface. This collector runs the error finding function on the received output and stores the found error lines.

@tothszabi tothszabi requested a review from godrei December 6, 2022 11:37
@tothszabi tothszabi merged commit d00ca1a into master Dec 9, 2022
@tothszabi tothszabi deleted the command-errors branch December 9, 2022 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants