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: return false for every entry is irrelevant #206

Merged
merged 2 commits into from May 4, 2018

Conversation

yinm
Copy link
Contributor

@yinm yinm commented Apr 28, 2018

I fix return value for matching spec (should return the all stack entries if every entry is irrelevant).

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@yinm
Copy link
Contributor Author

yinm commented Apr 28, 2018

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

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.

Please explain why this change is needed.

How can this test pass in both before and after your change?

@yinm
Copy link
Contributor Author

yinm commented May 4, 2018

Thank you for reply.

How can this test pass in both before and after your change?

In current master branch, there is no difference between should split by newline and return all values for which isExternalStackEntry returns true spec and should return the all stack entries if every entry is irrelevant spec.
Thus, test passes in both before and after my change.

This phenomenon occurred at yinm@fce80bf.
So, I reverted return value from true to false.

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!

@@ -465,7 +465,7 @@ describe('jasmine adapter', function () {
})

it('should return the all stack entries if every entry is irrelevant', function () {
spyOn(window, 'isExternalStackEntry').and.returnValue(true)
spyOn(window, 'isExternalStackEntry').and.returnValue(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment here as the two cases are confusing (which is why the bug was in the code). For example:

// Check the case where the filteredStack is empty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!
I committed at c25a0b8

@johnjbarton johnjbarton merged commit d7523d0 into karma-runner:master May 4, 2018
@yinm yinm deleted the fix-test branch July 5, 2018 02:44
willmendesneto pushed a commit to willmendesneto/ng-numbers-only that referenced this pull request Nov 16, 2018
## The devDependency [karma-jasmine](https://github.com/karma-runner/karma-jasmine) was updated from `1.1.2` to `2.0.0`.
This version is **not covered** by your **current version range**.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

---

<details>
<summary>Release Notes for v2.0.0</summary>

<ul>
<li>feat (adapter): Use jasmine's new configure method (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="375153235" data-permission-text="Issue title is private" data-url="karma-runner/karma-jasmine#224" data-hovercard-type="pull_request" data-hovercard-url="/karma-runner/karma-jasmine/pull/224/hovercard" href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/pull/224">#224</a>) (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/6663e47">6663e47</a>), closes <a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/issues/224" data-hovercard-type="pull_request" data-hovercard-url="/karma-runner/karma-jasmine/pull/224/hovercard">#224</a> <a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/issues/221" data-hovercard-type="issue" data-hovercard-url="/karma-runner/karma-jasmine/issues/221/hovercard">#221</a></li>
</ul>
<h3>Bug Fixes</h3>
<ul>
<li><strong>adapter:</strong> Remove incorrect function and its call. (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/issues/183" data-hovercard-type="pull_request" data-hovercard-url="/karma-runner/karma-jasmine/pull/183/hovercard">#183</a>) (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/cada4a9">cada4a9</a>)</li>
<li>return false for every entry is irrelevant (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/issues/206" data-hovercard-type="pull_request" data-hovercard-url="/karma-runner/karma-jasmine/pull/206/hovercard">#206</a>) (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/d7523d0">d7523d0</a>), closes <a href="https://urls.greenkeeper.io//github.com/karma-runner/karma-jasmine/pull/206/issues/discussion_r186142116">/github.com/karma-runner/karma-jasmine/pull/206#discussion_r186142116</a></li>
<li><strong>console:</strong> Re-add Error: to the stack (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/issues/228" data-hovercard-type="pull_request" data-hovercard-url="/karma-runner/karma-jasmine/pull/228/hovercard">#228</a>) (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/d0b980d">d0b980d</a>)</li>
<li><strong>time:</strong> report correct time since Jasmine v2.9.0 (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/issues/197" data-hovercard-type="pull_request" data-hovercard-url="/karma-runner/karma-jasmine/pull/197/hovercard">#197</a>) (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/022ee04">022ee04</a>), closes <a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/issues/196" data-hovercard-type="issue" data-hovercard-url="/karma-runner/karma-jasmine/issues/196/hovercard">#196</a></li>
</ul>
<h3>Chores</h3>
<ul>
<li><strong>deps:</strong> Drop node v4 support. (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/issues/214" data-hovercard-type="pull_request" data-hovercard-url="/karma-runner/karma-jasmine/pull/214/hovercard">#214</a>) (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/e604132">e604132</a>)</li>
</ul>
<h3>Features</h3>
<ul>
<li>Propagate errors thrown in afterAll blocks (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/f3fa264">f3fa264</a>), closes <a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/issues/161" data-hovercard-type="issue" data-hovercard-url="/karma-runner/karma-jasmine/issues/161/hovercard">#161</a></li>
<li>update the version to 2.0.0 and restrict node version available to 4.0 (<a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/c84316e">c84316e</a>)</li>
</ul>
<h3>BREAKING CHANGES</h3>
<ul>
<li><code>stopOnFailure</code>, which was previously documented in karma-jasmine's README, is<br>
not configuration option for jasmine. Use <code>oneFailurePerSpec</code> instead.</li>
</ul>
<p>Requires peerDependency Jasmine@^3.3.0</p>
<ul>
<li><strong>deps:</strong> Drop support for node 4.x</li>
</ul>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 9 commits.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/2dbec133017572933d02d67a88673d5d67fb227a"><code>2dbec13</code></a> <code>chore: release v2.0.0</code></li>
<li><a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/4ddfe2ffdf667c66eaddcc405c7042cdde685ddb"><code>4ddfe2f</code></a> <code>chore: update contributors</code></li>
<li><a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/d0b980db3cb363b7fb0cd48dcd7d529aac83fbca"><code>d0b980d</code></a> <code>fix(console): Re-add Error: to the stack (#228)</code></li>
<li><a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/6663e4733ad673472bfc4dae7c76d076370e5770"><code>6663e47</code></a> <code>feat (adapter): Use jasmine's new configure method (#224)</code></li>
<li><a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/e604132ce243f685b5547745b9739c420a294729"><code>e604132</code></a> <code>chore(deps): Drop node v4 support. (#214)</code></li>
<li><a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/66f598a5fd9b899f655e706576e725b02ccc3de1"><code>66f598a</code></a> <code>docs:(typo) timeoutInterval (#212)</code></li>
<li><a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/5b45cc12618bf6591bbf8fb3271f2c608927fceb"><code>5b45cc1</code></a> <code>Adding timeout configuration option (#211)</code></li>
<li><a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/d7523d0d89f89c8c4f454ddd83ef09da1858e207"><code>d7523d0</code></a> <code>fix: return false for every entry is irrelevant (#206)</code></li>
<li><a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/commit/3b20480a5a1cfa769c2a1861455c76d3bfaaab52"><code>3b20480</code></a> <code>--grep option now supports real regular expressions but not masks only. (#204)</code></li>
</ul>
<p>See the <a href="https://urls.greenkeeper.io/karma-runner/karma-jasmine/compare/b670e886dde410b6f28736985036e2ba6c74141e...2dbec133017572933d02d67a88673d5d67fb227a">full diff</a></p>
</details>

<details>
  <summary>FAQ and help</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>

---


Your [Greenkeeper](https://greenkeeper.io) bot 🌴
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.

None yet

3 participants