Closed (fixed)
Project:
Chatbot API
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
25 Sep 2017 at 05:24 UTC
Updated:
19 Oct 2017 at 14:20 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
gambryComment #3
larowlanShould be able to make this a kernel test if we add another test module that has a test view in it.
Comment #4
gambryYep, that's the plan. Also we should add a basic test handling the pager/iterator, althought I would like to improve and better document that bit.
Comment #5
gambryThis patch:
- Add configuration schema for views.display.chatbot_intent, required in order to run the test.
- Add a test module with a views with a chatbot_intent display.
- Add a test, asserting the views intent is loaded, processed and pager/iterator works.
Remaining tasks:
- Test doesn't cover potential filters coming from slots. This is because I noticed filters values are scanned in both context and slots even if those filters are not exposed. I'm not sure if this is a feature or a bug. Additionally value casting to FALSE - i.e. status = 0 - even if valid are skipped on
ViewsIntent::processFilters(). I think the filters processor needs a bit of love in a followup.- Assertion are inside closure. I like it, however it makes really difficult to debug in case of failure as phpunit prints out errors like "Fatal error: Uncaught Exception: Serialization of 'Closure' is not allowed in -:68". We may review those in here or a followup.
Comment #6
larowlanI think $this inside the closure still corresponds to the test class
other than that, looks good - agree the closure is painful, but can't think of another way
Comment #7
gambry@larowlan I tried with
$thisinside the closure, and got this error:The other way would be to define variables, passing to the closures as references and asserting them after the prophecies ran, i.e.
Code is slightly less clean, but phpunit outputs exception messages as you would expect.
Comment #8
gambryComment #10
gambryAs we need to make some progress in here I've committed #5 taking in consideration #6.1 (using directly
$thisin the closure) returns an error so there were no outstanding feedback.I still think the clueless error of asserting inside the closure is potentially dangerous. Solution in #7 works but I'm not sure how good practice is to pass "pawns" as reference to be changed within closure and asserting their values later.
We can always change it later.
Thanks!