Remove Order.sort function passing to Karma callback
#272
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
johnjbarton
suggested changes
Aug 11, 2020
src/adapter.js
Outdated
| @@ -191,8 +191,13 @@ function KarmaReporter (tc, jasmineEnv) { | |||
| // Any errors in top-level afterAll blocks are given here. | |||
| handleGlobalErrors(result) | |||
|
|
|||
| // Remove functions from called back results to avoid IPC errors in Electron | |||
| // https://github.com/twolfson/karma-electron/issues/47 | |||
| let cleanedOrder = Object.assign({}, result.order); | |||
There was a problem hiding this comment.
Looks like Object.assign() is not supported on IE11.
https://caniuse.com/#search=object.assign
I think we could loop over Object.keys() and copy if typeof result[key] !== 'function'.
|
👍 Taking today off so will revisit tomorrow
…On Tue, Aug 11, 2020, 9:35 AM johnjbarton ***@***.***> wrote:
***@***.**** requested changes on this pull request.
The commit message first line would be like
fix(adapter): filter functions from result
------------------------------
In src/adapter.js
<#272 (comment)>
:
> @@ -191,8 +191,13 @@ function KarmaReporter (tc, jasmineEnv) {
// Any errors in top-level afterAll blocks are given here.
handleGlobalErrors(result)
+ // Remove functions from called back results to avoid IPC errors in Electron
+ // twolfson/karma-electron#47
+ let cleanedOrder = Object.assign({}, result.order);
Looks like Object.assign() is not supported on IE11.
https://caniuse.com/#search=object.assign
I think we could loop over Object.keys() and copy if typeof result[key]
!== 'function'.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#272 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAG4KWHDEBFRUYSZBLENBVLSAFXMFANCNFSM4O3G6YRQ>
.
|
|
Oh wow, I somehow assumed this repo was using a transpiler somehow (usually avoid them myself). Going to also drop the |
|
Also using |
|
Alright, that should do it. Tested with repro repo here from original issue https://gist.github.com/twolfson/5122c4398147df6bb2bfaeb37a59997b I'll update the commit message now with what you gave me |
twolfson
force-pushed
the
dev/stop.passing.functions.in.callbacks
branch
from
07c9aa0
to
4b9f952
Compare
|
Decided to just squash the commits as I don't know how the commits are merged/squashed on this repo =/ |
johnjbarton
approved these changes
Aug 12, 2020
karmarunnerbot
pushed a commit
that referenced
this pull request
Aug 12, 2020
## [4.0.1](v4.0.0...v4.0.1) (2020-08-12) ### Bug Fixes * **adapter:** filter functions from result.order ([#272](#272)) ([28f337c](28f337c))
|
🎉 This PR is included in version 4.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Thanks for keeping on top of this @twolfson and @johnjbarton thanks for getting this merged. I'll be glad to remove our kludgey workaround for this! |
crapStone
pushed a commit
to Calciumdibromid/CaBr2
that referenced
this pull request
Apr 25, 2022
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [karma-jasmine](https://github.com/karma-runner/karma-jasmine) | devDependencies | major | [`4.0.2` -> `5.0.0`](https://renovatebot.com/diffs/npm/karma-jasmine/4.0.2/5.0.0) | --- ### Release Notes <details> <summary>karma-runner/karma-jasmine</summary> ### [`v5.0.0`](https://github.com/karma-runner/karma-jasmine/blob/HEAD/CHANGELOG.md#​500-httpsgithubcomkarma-runnerkarma-jasminecomparev402v500-2022-04-12) [Compare Source](karma-runner/karma-jasmine@v4.0.2...v5.0.0) ##### Bug Fixes - limit karma peer dependency to ^6.0.0 ([d72c124](karma-runner/karma-jasmine@d72c124)) ##### Build System - drop Node.js 10 support ([ea691e8](karma-runner/karma-jasmine@ea691e8)) ##### Features - **deps:** update dependencies including jasmine-core ([821f094](karma-runner/karma-jasmine@821f094)) ##### BREAKING CHANGES - The minimum required version of karma is 6.0.0. - The minimum required version of Node is 12.0.0. - **deps:** jasmine-core was updated to the 4.1.0. Please refer to the [release notes](https://github.com/jasmine/jasmine/blob/main/release_notes/4.0.0.md) for the complete list of changes and migration instructions. #### [4.0.2](karma-runner/karma-jasmine@v4.0.1...v4.0.2) (2022-03-30) ##### Bug Fixes - sync package-lock.json and package.json ([4dacc5d](karma-runner/karma-jasmine@4dacc5d)) #### [4.0.1](karma-runner/karma-jasmine@v4.0.0...v4.0.1) (2020-08-12) ##### Bug Fixes - **adapter:** filter functions from result.order ([#​272](karma-runner/karma-jasmine#272)) ([28f337c](karma-runner/karma-jasmine@28f337c)) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1302 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We received a bug report in
karma-electron, twolfson/karma-electron#47, which eventually traced back to Electron being unable to pass afunctionto between Karma contexts due to IPC fixing passing references between processesThis PR resolve that bug by removing the
functionwhich was being attempted to pass as a reference between processesIn this PR:
Order.sortfunction passing to Karma callbackFeel free to request commit message changes (or apply them yourself). I tried to find documentation on them but was running out of time =/