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

Expose 'disabled' and 'pending' statuses to let reporter differentiate them #129

Closed
segrey opened this issue Aug 9, 2016 · 7 comments
Closed

Comments

@segrey
Copy link

segrey commented Aug 9, 2016

Jasmine exposes 'disabled' and 'pending' statuses via specResult.status.

IIUC, specResult.disabled === true means that there is at least one focused spec (fdescribe/fit) and this spec is not declared as focused.

specResult.pending === true means that this spec is declared as pending (xdescribe/xit or without a function body).

Currently, karma-jasmine merges both these statuses into skipped (https://github.com/karma-runner/karma-jasmine/blob/v1.0.2/src/adapter.js#L214) making it impossible for a reporter to handle these statuses differently. E.g. for IDE reporter, it would be convenient to show pending specs in a result test tree conditionally (e.g. if "Show Pending" is enabled) and hide disabled spec always.

Would be great if a reporter could access result.disabled and result.pending values in onSpecComplete(browser, result).
Thanks!

@segrey
Copy link
Author

segrey commented Aug 9, 2016

Depending issue in WebStorm: https://youtrack.jetbrains.com/issue/WEB-22696

@segrey segrey changed the title Expose 'disabled' and 'pending' status to let reporter differentiate them Expose 'disabled' and 'pending' statuses to let reporter differentiate them Aug 9, 2016
@maksimr
Copy link
Contributor

maksimr commented Aug 9, 2016

@segrey what about add only one value status?

@segrey
Copy link
Author

segrey commented Aug 9, 2016

@maksimr no particular preferences here. Any way that allows to differentiate 'disabled' and 'pending' specs would go. As for having status, IIUC, it would cover success property as well. So, result.skipped, result.success, result.disabled and result.pending boolean properties could be replaced with result.status string property. The only downside IMO is that it's easier/clearer to check boolean fields than to compare strings.

@maksimr
Copy link
Contributor

maksimr commented Aug 10, 2016

But it's better if sometimes in future jasmine will add new status we should not change the source code.

@segrey
Copy link
Author

segrey commented Aug 10, 2016

@maksimr if I understand you correctly, you suggest to have result.status = specResult.status in https://github.com/karma-runner/karma-jasmine/blob/v1.0.2/src/adapter.js#L214? If yes, then unfortunately, in this case, result.status values won't be unified among other adapters (i.e. result.status property would make sense for karma-jasmine adapter only).
I think this problem can be (and should be) approached in test-framework agnostic way, because disabled and pending looks like common test statuses, so other test frameworks (e.g. mocha) could benefit.
IMO, a reporter should depend on karma only, and karma abstracts out all differences between test frameworks via corresponding adapters.

@segrey
Copy link
Author

segrey commented Aug 10, 2016

As for in what way karma API should be extended, this question is still open for me. Options:

  • add result.disabled and result.pending boolean properties
  • or add result.status string property with values 'disabled', 'pending', 'failed', 'success' (making result.success boolean property obsolete).

I'd slightly lean to the first option as it's a smaller API change. Other options and thoughts are welcome.
@maksimr @dignifiedquire

@maksimr maksimr self-assigned this Aug 18, 2016
maksimr added a commit that referenced this issue Aug 19, 2016
feat: report status of the spec closes #129
@segrey
Copy link
Author

segrey commented Nov 10, 2016

Thanks for the fix, now result.pending is used by karma-runner/karma-intellij@b5a39e3
Currently, result.disabled is unused. Probably, I was wrong requesting this field to be exposed too.
Should it be removed for easier maintenance?
Maybe some documentation is needed on this matter that would describe result.skipped and result.pending fields.
@maksimr Thanks a lot for these fixes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants