Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ios): add xcode timeout result parsing #985

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

msd117c
Copy link

@msd117c msd117c commented Nov 21, 2024

The TestRunProgressParser is reading the xcrun logs to detect the end time and calculate the start time. However, when a timeout is defined from XCode and this timeout is triggered, the log looks like:

Test Case '-[TestTarget.TestClass test]' exceeded execution time allowance of 3 minutes. The test may have hung; check Xcode's test report for additional diagnostics. If the test requires additional time, use -[XCTestCase executionTimeAllowance] or one of xcodebuild's command-line overrides.

Resulting on the parser not being able to correctly identified what happened.

This PR implements an extra case in the TestRunProgressParser to match this case and fix the issue reported here

@Malinskiy
Copy link
Member

Thanks for sending this.
I think it's a good addition to handling edge cases for Apple

Can you please double check if timeout case produces 0 value in the TemporalTestResult please?
There is also a piece of logic here that is supposed to fill in those 0's, I wonder why it's not invoked, assuming this is the source your issue with negative durations.

@msd117c
Copy link
Author

msd117c commented Nov 22, 2024

Thanks for sending this. I think it's a good addition to handling edge cases for Apple

Can you please double check if timeout case produces 0 value in the TemporalTestResult please? There is also a piece of logic here that is supposed to fill in those 0's, I wonder why it's not invoked, assuming this is the source your issue with negative durations.

Ok, I have been running some tests and debugging the solution. I did not finish but wanted to register the progress here and let you know the status.

TemporalTestResult

Yes, this use case produces a value of 0 for the endTime property.

The problem seems to be in the TestRunProgressParser. Without the check I added it does not detect the test finished. This results into the AppleSimulatorDevice not detecting the test finished event (it does not exist).

...
for (line in session.stdout) {
    testEventProducer.process(line)?.let {
        send(it)
    } 
    lineListeners.forEach { it.onLine(line) }
}
...

The test is then detected as uncompleted and the TestsResultListener process this case here:

if (uncompleted.isNotEmpty()) {
    uncompleted.forEach {
        logger.warn { "uncompleted = ${it.test.toTestName()}, ${device.serialNumber}" }
    }
}

There I could print the endTime property (being 0).

Fill endTime strategy

Here is where I need to keep testing. I log that function and is not being invoked but I could not determine why that is happening.

Regarding the timeout from Apple, more information in Apple docs. Basically it is specified in seconds and it is round up to minutes so we can expect to use (in regex) minutes*.

How to reproduce

  • Create a XCUITest with a delay of 61 seconds.
  • For the vendor configuration, enable the timeout for the minimum (1 minute):
      xcodebuildTestArgs:
        "-test-timeouts-enabled": "YES"
        "-maximum-test-execution-time-allowance": "60"
  • The test should be reported as non completed and the log:
    "Test $pkg.$clazz.$method finished with result <$result> after $duration seconds"
    should be missing.

@msd117c
Copy link
Author

msd117c commented Nov 25, 2024

After checking again, it seems the fillEndTime function is not used because it is a non completed test

@msd117c msd117c marked this pull request as ready for review November 25, 2024 11:15
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.

2 participants