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(runner): Fix getting result by config #3381

Conversation

ilgonmic
Copy link
Contributor

Since Karma 4.4.0, there is problem with exit code with failOn* properties, because now getResults expect only config, but emitting include concrete properties from config.
So it seems that all the failOn* properties don't work in 4.4.0

@AppVeyorBot
Copy link

Build karma 2421 failed (commit fd31579fd2 by @ilgonmic)

@karmarunnerbot
Copy link
Member

Build karma 23 completed (commit fd31579fd2 by @ilgonmic)

@karmarunnerbot
Copy link
Member

Build karma 22 completed (commit fd31579fd2 by @ilgonmic)

Copy link
Contributor

@johnjbarton johnjbarton left a comment

Choose a reason for hiding this comment

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

Thanks! Can you add a test to prevent such a fail? I wonder why our tests did not catch it.

@johnjbarton
Copy link
Contributor

Looks there is no unit test coverage for that function

@ilgonmic
Copy link
Contributor Author

I’ll watch on this and try to cover these cases

@johnjbarton
Copy link
Contributor

I think I have coverage for this case now. just getting that last case to pass....

@johnjbarton
Copy link
Contributor

I added a test that covers this case thus also your fix, PR #3384 , please review.

I'm happy to comment out the test and commit if you want to land this PR.

@ilgonmic
Copy link
Contributor Author

@johnjbarton It does not matter for me :)
I think, I can close this PR in favor of #3384

@ilgonmic ilgonmic closed this Oct 18, 2019
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.

4 participants