Opened 5 years ago

Closed 5 years ago

#11992 closed Task (fixed)

Porting tests to bender and moving to the dev repor

Reported by: Piotr Jasiun Owned by: Piotr Jasiun
Priority: Normal Milestone: CKEditor 4.4.2
Component: General Version:
Keywords: Cc:

Description

CKEditor tests should be ported to the Bender and moved to the main CKEdior project.

Change History (11)

comment:1 Changed 5 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: newassigned

comment:2 Changed 5 years ago by Piotr Jasiun

Related future ticket: #12004.

comment:3 Changed 5 years ago by Piotr Jasiun

Cleanup in the test project and the porting scripts are done. Now we need to close tests project (so there will be no new tests), convert everything to bender and then fix converted tests, because it is not possible to fix everything automatically.

It is good moment for a partial review.

What is done in the test project (branch t/11992):

  • everything what is not a test is moved to the _assets, _helpers or _docs folder,
  • paths are fixed,
  • html code is moved to the body,
  • some minor fixes.

What porting scripts do:

  • code is separated to html and js files,
  • tags are transformed,
  • body tag is removed if it's not needed and keep if it is needed (if there are attributes or head),
  • indentation in transformed html is fixed,
  • unneeded empty lines are removed,
  • code style in js files is fixed,
  • tests/tests (dt/tests) folder is removed,
  • empty html files are removed,
  • some paths are fixed.

Changes in dev project:

  • bender.js and package.json files are added (available in the t/11992 branch),
  • example of ported tests is available in the t/11992-tests branch; this branch is only an examples of how porting works; changes in this branch will be removed and replaces with the newest tests; note that there are about 50 not passing tests and bender stops on tests/tickets/10913/1 and tests/tickets/11372/1 because of wrong paths.

comment:4 Changed 5 years ago by Piotr Jasiun

Status: assignedreview

comment:5 Changed 5 years ago by Piotrek Koszuliński

  1. t/11992:
  • breaks .gitignore by removing dev/iconmaker/node_modules.
  • package.json is incorrect. It has to be generalised for the entire ckeditor-dev.
  • array of files ignored by builder has to be fixed.
  1. t/11992-tests:
  • tests/_helpers/pasting.js is a clipboard's helper.
  • code style fixer should touch only tests' files.
  • there's much more problems with code style in our test files... I think that we don't have time to work on all of them now, but maybe one additional thing: space before colon. It may be hard to find all this by regexp, so we can make the regexp more specific: / : function/: function/
  • all these paths are unnecessarily long:
    tickets/8103/1.html:	<script src="../../tests/_helpers/pasting.js"></script>
    tickets/9330/1.html:	<script src="../../tests/_helpers/pasting.js"></script>
    tickets/11237/1.html:	<script src="../../tests/_helpers/pasting.js"></script>
    tickets/11372/1.html:	<script src="../../tests/plugins/widget/_helpers/tools.js"></script>
    tickets/9456/2.html:	<script src="../../tests/_helpers/pasting.js"></script>
    tickets/9456/1.html:	<script src="../../tests/_helpers/pasting.js"></script>
    

comment:6 Changed 5 years ago by Piotr Jasiun

Status: reviewassigned

comment:7 Changed 5 years ago by Piotr Jasiun

Red tests exported to separate tickets: #12040.

comment:8 Changed 5 years ago by Piotr Jasiun

All tests are green now (at least on Chrome).

I compared console log from bender and CKTester and most of the new messages are because of Bender bugs (#29 and #30), but there are some messages need investigation:

http://127.0.0.1:1030/tests/tests/core/ckeditor/basepath

GET http://localhost:1030/ckeditor/core/loader.js 404 (Not Found) ckeditor.js:24
GET http://localhost:1030/tests/ckeditor/core/loader.js 404 (Not Found) ckeditor.js:24
Uncaught [CKEDITOR.resourceManager.load] Resource name "default" was not found at "http://localhost:1030/apps/ckeditor/styles.js". resourcemanager.js:211
Uncaught [CKEDITOR.resourceManager.load] Resource name "default" was not found at "http://localhost:1030/apps/ckeditor/styles.js". resourcemanager.js:211

(fixed) http://127.0.0.1:1030/tests/tests/core/dom/document

GET http://localhost:1030/apps/ckeditor/samples/_assets/sample.css 404 (Not Found) element.js:216
yui: NOT loaded: loader-base yui-3.17.1-dom-screen-node-core.min.js:14
yui: NOT loaded: loader-rollup yui-3.17.1-dom-screen-node-core.min.js:14
yui: NOT loaded: loader-yui3 yui-3.17.1-dom-screen-node-core.min.js:14
yui: NOT loaded: node-base yui-3.17.1-dom-screen-node-core.min.js:14
yui: NOT loaded: node-event-delegate yui-3.17.1-dom-screen-node-core.min.js:14
yui: NOT loaded: node-pluginhost yui-3.17.1-dom-screen-node-core.min.js:14
yui: NOT loaded: node-screen yui-3.17.1-dom-screen-node-core.min.js:14
yui: NOT loaded: node-style yui-3.17.1-dom-screen-node-core.min.js:14
Uncaught TypeError: undefined is not a function element:750
Last edited 5 years ago by Artur Delura (previous) (diff)

comment:9 in reply to:  8 ; Changed 5 years ago by Artur Delura

Related to:

GET http://localhost:1030/apps/ckeditor/samples/_assets/sample.css 404 (Not Found) element.js:216

I found out that error occur in test: http://localhost:1030/tests/tests/core/dom/document test_appendStyleSheet. There is hardcoded link url:

CKEDITOR.basePath + 'samples/_assets/sample.css';

This file is located in: https://github.com/ckeditor/ckeditor-dev/blob/master/samples/assets/sample.css I don't know why in hardocded url there is underscore as a prefix for assets directory (that is why there is an 404 response). So I changed hardocded url to

CKEDITOR.basePath + 'tests/_assets/sample.css';

because there is such file in tests/assets directory. And everything works well, but I need confirmation that I can remove this file: https://github.com/ckeditor/ckeditor-dev/blob/master/samples/assets/sample.css . I think it can't break nothing, but it's outside tests area, so I'm cautious.

comment:10 in reply to:  9 Changed 5 years ago by Piotrek Koszuliński

Replying to a.delura:

I think it can't break nothing, but it's outside tests area, so I'm cautious.

Wow... I had to be drunk to put it there. You can remove it.

comment:11 Changed 5 years ago by Piotrek Koszuliński

Resolution: fixed
Status: assignedclosed

Done! I added changelog entry in git:95b5607.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy