Closed (fixed)
Project:
Collect
Version:
8.x-1.x-dev
Component:
Miscellaneous
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
2 Jul 2015 at 09:55 UTC
Updated:
1 Sep 2015 at 13:54 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
mbovan commentedComment #2
juanse254 commentedShould address the issue about front page not showing up but the test should be a different issue.
Comment #3
miro_dietikerThis patch will not apply. Meanwhile i have pushed something.
Pull and merge first.
Comment #4
mbovan commentedSame as (1) in https://www.drupal.org/node/2472479#comment-10082650.
Comment #5
juanse254 commentedJust rebased.
Comment #6
miro_dietikerI just wanted to commit, but this is a nice nitpick.
Ah yeah, what?
Comment #7
juanse254 commentedFixed this about the comment, also improved the url to be dynamic for the test.
Comment #8
mbovan commentedThis test is not actually tested (check test results). That's the reason why we get green tests.
Wrong path.
The namespace should be
Drupal\collect_demo\Tests.The host parameter is not present here.
View with capital "V".
Http code should be passed as an integer.
Extra double quote character in 'collect:collectjson/@host/entity/user"'
We don't really test anything here.
Unrelated change?
Comment #9
miro_dietikerWe do. It would report / throw an exception/error, if it fails. Also the response code is checked below. Which would fail in this issue with a test-only patch.
Comment #10
juanse254 commentedFixed with suggestions on comment #8.
Comment #12
mbovan commentedCreated an issue for broken ProcessingWebTest #2527148: Adding processors to the model is broken
The one of the reasons why CollectDemoTest failed is because CRM Core module is not on d.o (collect_demo has a dependency on it), so we cannot run the test properly...
I suggest to upload 2 patches, one with the leading change so we can commit that and the other one that will contain only CollectDemoTest (Named something like demo_article_not_displayed-2519966-12-DO-NOT-TEST.patch) so we can add the tests once CRM Core is on d.o.
P.S. We can still do "8" from the comment #8.
Comment #13
miro_dietikerThose are not translatable! It's just a placeholder replacement.
And yeah still unrelated change.
Comment #14
mbovan commentedAs #2527148: Adding processors to the model is broken is committed, Collect is green again. We can continue to work on this issue. Retesting...
Comment #17
juanse254 commentedComment #19
mbovan commentedCool!
We can commit the patch, and continue to work on fixing CollectDemoTest locally.
Also, we can open an (postponed) issue (similar to #2430111: Re-add tests when CRM Core is on drupal.org) and upload fixed CollectDemoTest there.
Comment #20
miro_dietikerNot really. The fixing patch does not contain the test!
Comment #21
miro_dietikerProviding combined patch for testbot.
Comment #23
miro_dietikerOK... The tests also don't pass locally because of some Schema config error. Let's fix this first.
Comment #24
miro_dietikerNote: Editing the core site information even throws an exception when data is faulty... See admin/config/system/site-information
Comment #27
miro_dietikerCommitted the fix. Tests can't work as long as crm core is not on d.o.
Postponing the demo test for crm core.
Comment #29
mbovan commentedMade some changes in CollectDemoTest. It should pass now with the patch #33 from #2535094: Schema missing when testing on collect_demo which depends on several modules.
Comment #30
mbovan commentedTest is moved into #2430111: Re-add tests when CRM Core is on drupal.org so we can mark this issue as fixed.