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

Remove Order.sort function passing to Karma callback #272

Conversation

twolfson
Copy link
Contributor

We received a bug report in karma-electron, twolfson/karma-electron#47, which eventually traced back to Electron being unable to pass a function to between Karma contexts due to IPC fixing passing references between processes

This PR resolve that bug by removing the function which was being attempted to pass as a reference between processes

In this PR:

Feel free to request commit message changes (or apply them yourself). I tried to find documentation on them but was running out of time =/

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.

The commit message first line would be like
fix(adapter): filter functions from result

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);
Copy link
Contributor

Choose a reason for hiding this comment

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

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'.

@twolfson
Copy link
Contributor Author

twolfson commented Aug 11, 2020 via email

@twolfson
Copy link
Contributor Author

Oh wow, I somehow assumed this repo was using a transpiler somehow (usually avoid them myself). Going to also drop the let to avoid browser compatibility issues

@twolfson
Copy link
Contributor Author

Also using getOwnPropertyNames as keys can iterate over prototype keys as well (which I believe we want to avoid)

@twolfson
Copy link
Contributor Author

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 twolfson force-pushed the dev/stop.passing.functions.in.callbacks branch from 07c9aa0 to 4b9f952 Compare August 12, 2020 22:03
@twolfson
Copy link
Contributor Author

Decided to just squash the commits as I don't know how the commits are merged/squashed on this repo =/

@johnjbarton johnjbarton merged commit 28f337c into karma-runner:master 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))
@karmarunnerbot
Copy link
Member

🎉 This PR is included in version 4.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@twolfson twolfson deleted the dev/stop.passing.functions.in.callbacks branch August 12, 2020 23:36
@dpwatrous
Copy link

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#&#8203;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 ([#&#8203;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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Karma times out when using Electron 9 and client.useIframe = false
4 participants