Skip to content
This repository has been archived by the owner on Dec 7, 2019. It is now read-only.

Updated handling of crashed application during test run. #130

Merged
merged 1 commit into from
Apr 3, 2018

Conversation

yunikkk
Copy link
Contributor

@yunikkk yunikkk commented Mar 6, 2018

Basically, thats it, fixes #121)

It appears that crashes are reported differently under different OS revisions. I've got another example with shortMsg=Native crash on Lollipop, too. So the common thing here is shortMsg and it should be sufficient to treat it as crash.

With this PR test run will fail, but the excluded tests will remain silently ignored.
Seems to me that possible nicer option would be to refactor error handling the way that other tests are reported as ignored. On the other hand it will be confusing, too.

IMO we can live with such solution until we integrate test orchestrator or our own test runner which will run tests independently and allow to restart crashed application and failed test.

WDYT @artem-zinnatullin ?

@artem-zinnatullin
Copy link
Collaborator

the way that other tests are reported as ignored. On the other hand it will be confusing, too.

Yeah, I wouldn't mess with that. Ignored means that they were ignored in the code, ie with @Ignore

@@ -61,7 +63,7 @@ private fun String.parseInstrumentationStatusValue(key: String): String = this
.trim()

private fun String.throwIfError(output: File) = when {
contains("INSTRUMENTATION_RESULT: shortMsg=Process crashed") -> {
contains("INSTRUMENTATION_RESULT: shortMsg=") -> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

lol, ok

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add a test though?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep

@yunikkk yunikkk force-pushed the dont-exit-if-crashed branch from a5b481b to f455738 Compare March 8, 2018 20:06
@yunikkk
Copy link
Contributor Author

yunikkk commented Mar 13, 2018

Hey @artem-zinnatullin , lets merge this?

Copy link
Collaborator

@artem-zinnatullin artem-zinnatullin left a comment

Choose a reason for hiding this comment

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

Noice

@artem-zinnatullin
Copy link
Collaborator

Yeah sorry, I think GitHub stopped sending emails when you push new commits :(

@yunikkk yunikkk merged commit c095a4a into gojuno:master Apr 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

One test case is not executed and exclude from HTML report
2 participants