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

feat: report status of the spec closes #129 #132

Merged
merged 1 commit into from
Aug 19, 2016
Merged

feat: report status of the spec closes #129 #132

merged 1 commit into from
Aug 19, 2016

Conversation

maksimr
Copy link
Contributor

@maksimr maksimr commented Aug 11, 2016

@dignifiedquire
Copy link
Member

Looks good to me, @segrey please let us know if this works for you

@segrey
Copy link

segrey commented Aug 11, 2016

This works for me as well. Maybe just a small improvement: let's rename result.status property to result.jasmineStatus to indicate that this property is exposed directly from jasmine (thus jasmine docs should be consulted to clarify the details) and other adapters won't have it.
[Offtopic] how about introducing disabled and pending statuses on karma level API? That would make it test framework agnostic simplifying reporter's life :). E.g. mocha adapter could also provide these statuses I guess.

@maksimr
Copy link
Contributor Author

maksimr commented Aug 11, 2016

@segrey does mocha have statuses 'disabled' and 'pending'?

@segrey
Copy link

segrey commented Aug 11, 2016

@maksimr ah, it turned out that things are different for mocha. Mocha has pending tests which have test.pending status and are reported as result.skipped (https://github.com/karma-runner/karma-mocha/blob/v1.1.1/src/adapter.js#L110). However, mocha doesn't report tests with 'disabled' status, which are all tests other than exclusive. Thus, one can say that mocha has disabled status, but it just doesn't report it :)
Since karma-intellij silently ignores all skipped tests then currently pending mocha tests are not shown in a result test tree. Will file a separate issue in https://github.com/karma-runner/karma-mocha/ regarding reporting pending status for mocha tests.

@segrey
Copy link

segrey commented Aug 11, 2016

As for the introduced result.status, taking into account its general name, not sure I understand in which way result.status should be treated by a reporter:

  • It is an internal jasmine status which can be changed in a subsequent jasmine release. Reporter may use it at its own risk. Also, result.status is defined for karma-adapter only. In this case, result.jasmineStatus suites better.
  • It is a part of karma API and result.status values are unified between different adapters. If a test framework changes it's internal status format (e.g. for jasmine, specResult.status may be changed to 'skipped' instead of 'disabled'), a corresponding adapter is updated and unchanged result.status values is maintained.

@segrey
Copy link

segrey commented Aug 11, 2016

Will file a separate issue in https://github.com/karma-runner/karma-mocha/ regarding reporting pending status for mocha tests.

Done karma-runner/karma-mocha#109

@maksimr
Copy link
Contributor Author

maksimr commented Aug 19, 2016

@segrey I have updated changes, check plz

@segrey
Copy link

segrey commented Aug 19, 2016

@maksimr LGTM, thank you

@maksimr maksimr merged commit 7578f91 into karma-runner:master Aug 19, 2016
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