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 bug where before/afterAll were being executed in disabled suites. #1225

Merged
merged 1 commit into from
Dec 2, 2016

Conversation

voithos
Copy link
Contributor

@voithos voithos commented Oct 24, 2016

This fixes #1175. This change also removes all the disabled-related code from Suite objects, including the .disable() method (I couldn't find any examples of e.g. emitting deprecation warnings, so I just removed it; let me know if there is something else that should be done here).

Note that this now seems to not run the specs inside a disabled suite, whereas in the past they ran but were marked 'pending' (was this a symptom of the bug?).

@slackersoft
Copy link
Member

The running of the specs and marking them pending is probably the source of this beforeAll/afterAll bug. We didn't want to change the reporting of specs within an xdescribe from previous behavior, but didn't notice the beforeAlls being executed. It looks like the TreeProcessor should be handling this case, but it would be good to get an integration spec that proves it.

Thanks.

@voithos
Copy link
Contributor Author

voithos commented Nov 8, 2016

Thanks for the response! It looks like there's already an it("does not run beforeAlls or afterAlls for a disabled node") spec in TreeProcessorSpec.js, and I had added an it("shouldn't run before/after functions in disabled suites") integration spec in integration/SpecRunningSpec.js to test it at a higher level. Do these not suffice?

@patthiel
Copy link

@slackersoft are we any closer to seeing this get merged? It adds a lot of unnecessary execution time to our tests.

@slackersoft slackersoft merged commit c21bdaf into jasmine:master Dec 2, 2016
slackersoft pushed a commit that referenced this pull request Dec 2, 2016
…asmine into voithos-beforeall-in-xdescribe

- Merges #1225 from @voithos
- Fixes #1175
@patthiel
Copy link

patthiel commented Dec 4, 2016

Thanks @voithos , @slackersoft !

@qualityshepherd
Copy link

This is still an issue in v2.5.3, which is included in Protractor 5.1.1. Thanks for fixing this... hope it releases soon!

@shavo007
Copy link

HI ,

I have this issue aswell.

jasmine-core is ^2.3.4

What version was this resolved in?

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.

beforeAll and afterAll execute asyncronious functions inside xdescribe, unlike their -Each counterparts
6 participants