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

Changes in 3.1.0 break custom karma-jasmine-html-reporter spec filtering #256

Closed
Sayan751 opened this issue Jan 13, 2020 · 20 comments · Fixed by #270
Closed

Changes in 3.1.0 break custom karma-jasmine-html-reporter spec filtering #256

Sayan751 opened this issue Jan 13, 2020 · 20 comments · Fixed by #270
Labels

Comments

@Sayan751
Copy link

As the latest version does a strict equality comparison, the filtering with suite name does not work anymore.

I am try to filter a suite from browser using the url http://localhost:9876/debug.html?spec=MySuite or by clicking the generated suite link (same url). It results in following error.

adapter.js:344 Uncaught Error: No spec found with name: "MySuite"
    at getSpecsByName (adapter.js:344)
    at getDebugSpecToRun (adapter.js:354)
    at getSpecsToRun (adapter.js:429)
    at new KarmaSpecFilter (adapter.js:435)
    at createSpecFilter (adapter.js:452)
    at ContextKarma.start (adapter.js:479)
    at ContextKarma.window.__karma__.loaded (debug.js:27)
    at debug.html?spec=MySuite:53

Ideally, the generated links should work.

@johnjbarton
Copy link
Contributor

Please provide steps to reproduce. For example, a small testsuite that fails.

@johnjbarton
Copy link
Contributor

Perhaps you mean: the filtering by spec name should be "grep"-like rather than exact?

@Sayan751
Copy link
Author

Sayan751 commented Jan 13, 2020

What I meant is that if there are following suites, then http://localhost:9876/debug.html?spec=MySuite fails to run all the specs from the suite MySuite.

describe('MySuite', function(){
 it("works1", function(){...});
 it("works2", function(){...});
});

describe('FooBar', function(){
 it("works1", function(){...});
 it("works2", function(){...});
});

AFAIK, previously the match was resolved by using a regex. With the new update the match is done by strictly comparing 2 strings. This does not work as the items in specs list has name in following format: ${suiteName} ${specName}. Thus, naturally the comparison fails when the spec name (from the querystring of the generated URL) contains the suite name.

@mpaluchowski
Copy link

I think this also broke running individual specs in IntelliJ IDEA / Webstorm, which specifies suite names in the form of regex patterns, ie. ^reservation-form component customer autocomplete .

@johnjbarton
Copy link
Contributor

Ok so I can imagine that my change broke you, but I can't understand how.

As far as I can tell, the 2.0.1 version did not support debug URL based test selection:
https://github.com/karma-runner/karma-jasmine/blob/61d15f1719ab5da0141dae5a2ec01519508c36de/src/adapter.js
The code does not even read window.location.

Is it possible that you have some additional code that is providing this URL->filter feature?

The 3.1.0 feature was based on writing the fullname onto the URL and thus does not need regexp. I'm ok with trying to add that as a new feature if we agree on what it should do.

@mpaluchowski
Copy link

Thanks for that. IntelliJ runs tests by executing a command similar to this:

node /home/michal/.IntelliJIdea2019.3/config/plugins/js-karma/js_reporter/karma-intellij/lib/intellijRunner.js --serverPort=9876 --protocol=http: --urlRoot=/ "--testName=^reservation-form component customer autocomplete "

where intellijRunner.js is part of the IDE's Karma plugin. The specific file is located here.

@Sayan751
Copy link
Author

@johnjbarton Sorry my bad. I am using this for so long that the lines have blurred :P

I am also using karma-jasmine-html-reporter which generates the links for the Suites as http://localhost:9876/debug.html?spec=MySuite. The new change of exact comparison from this package broke that. As many other packages (I assume) rely on this, do you think you can bring back the partial comparison?

@johnjbarton
Copy link
Contributor

Ok, now we are making progress. karma-jasmine-html-reporter provides spec filtering:
https://github.com/dfederm/karma-jasmine-html-reporter/blob/master/src/lib/adapter.js#L67

do you think you can bring back the partial comparison?

No, because the previous code did not implement partial comparison, it was implemented by jasmine.HtmlSpecFilter() as you can see from the code in karma-jasmine-html-reporter linked above.

So something in the 3.1 change breaks specFiltering, but you are in a much better position to figure out what that is. Since karma-jasmine did not have partial comparison code I can't bring that back.

@johnjbarton johnjbarton changed the title Filtering suites with name is not possible in 3.1.0 Changes in 3.1.0 somehow break karma-jasmine-html-reporter spec filtering Jan 14, 2020
@tscislo
Copy link

tscislo commented Feb 2, 2020

I reverted back to
"karma-jasmine": "~3.0.3"
and all works fine in WebStorm
Please fix the newest version as well.

@nbaki
Copy link

nbaki commented Feb 18, 2020

I have reverted my karma-jasmine to 3.0.3 but still getting the error.
I have my karma-jasmine-html-reporter at 1.5.2

@dfederm
Copy link

dfederm commented Mar 6, 2020

@johnjbarton adapter.js (the thing that does the spec filtering) from karma-jasmine-html-reporter is basically just a copy of boot.js from jasmine-core. All karma-jasmine-html-reporter really does is wrap jasmine's html reporter in something karma understands. So this could be seen as part of jasmine. It's my opinion that karma-jasmine should not break core jasmine functionality.

@simondel
Copy link

It seems like the order in which specFilter is configured has been changed. In 3.0.3, the specFilter from karma-jasmine is set before my custom specFilter. In 3.1.0, this is the other way around, which results in my specFilter never getting executed.

@simondel
Copy link

@johnjbarton The specFilter is now set in createStartFn so it's set at window.__karma__.start instead of when adapter.js as a file is loaded (previous behavior). This causes all other use cases of specFilter to be overruled by karma-jasmine.

What's the reason for setting the specFilter at that stage in the process? Would it be possible to either set the specFilter at the previous stage in the process or to not override the specFilter if a custom one has been set?

@johnjbarton
Copy link
Contributor

What's the reason for setting the specFilter at that stage in the process?

Well to be sure the reason is simple: no test fails when I made that change.

@johnjbarton johnjbarton changed the title Changes in 3.1.0 somehow break karma-jasmine-html-reporter spec filtering Changes in 3.1.0 break custom karma-jasmine-html-reporter spec filtering Mar 16, 2020
@MarcusRiemer
Copy link

I can confirm that karma-jasmine: 3.0.3 together with karma: 4.4.1 works as expected: If I click a section created by describe() in the HTML-runner I get a URL with a partial path and all specs that are a subset of that path are executed:

screenrecord-filter-works

With karma-jasmine: 3.1.1 I only get the Error: No spec found with name: "OverviewBlockLanguageComponent" output in the dev console:

screenrecord-filter-borked

@nbaki
Copy link

nbaki commented Apr 20, 2020

@MarcusRiemer That worked for me. Thanks!

@nicojs
Copy link
Contributor

nicojs commented Apr 23, 2020

What's the reason for setting the specFilter at that stage in the process?

Well to be sure the reason is simple: no test fails when I made that change.

@johnjbarton that's fair. Would you be willing to accept a PR that places the code back in adapter.js and adds a test for a spec filter override?

Thanks for the awesome work maintaining Karma. It's a very important development tool. ❤

@johnjbarton
Copy link
Contributor

Good PRs are always welcome as are more tests.

johnjbarton added a commit that referenced this issue May 28, 2020
jasmine tests are named suite-name + spec-name. The links printed by
karma-jasmine-html-reporter include links for suite-name (only). To support
running the suite, allow the filter to match any part of the test name.

Fixes #256
karmarunnerbot pushed a commit that referenced this issue May 28, 2020
## [3.3.1](v3.3.0...v3.3.1) (2020-05-28)

### Bug Fixes

* **filter:** match any portion of a spec name ([#270](#270)) ([ded4c4b](ded4c4b)), closes [#256](#256)
@karmarunnerbot
Copy link
Member

🎉 This issue has been resolved in version 3.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

parloti added a commit to parloti/karma-jasmine that referenced this issue May 31, 2020
jasmine tests are named suite-name + spec-name. The links printed by
karma-jasmine-html-reporter include links for suite-name (only). To support
running the suite, allow the filter to match ONLY the START portion of the test name.
Fixes karma-runner#256 karma-runner#270
@parloti
Copy link

parloti commented May 31, 2020

#270
To support running the suite, allow the filter to match any part of the test name.

I think the correspondence should only be for the start portion of the test name.
The released solution makes it difficult to filter test sections.
If I have the following situation:

test1
  subTest
test2
  subTest

And open test1 and then the corresponding subTest, I will have both the subTest, but it would be intuitive to see only the subTest correpondent to test1.

I have already sent a change proposal through pr #271, if that seems appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment