ckeditor / ckeditor5-design

☠ Early discussions about CKEditor 5's architecture. Closed now. Go to https://github.com/ckeditor/ckeditor5 ☠
58 stars 12 forks source link

Proposal: Wrap all tests in a meaningful describe() #137

Closed Reinmar closed 8 years ago

Reinmar commented 8 years ago

I've noticed that sometimes we wrap the whole test file with a meaningful describe() and sometimes we only call describe() for methods and properties. Wrapping has one major advantage – when running tests in Node.js it's clear what failed. When the test file isn't wrapped, you get some meaningless method/properties names and no context.

So, let's wrap all files. It does not need to be one describe(), because a test file may test multiple subjects, but the most outer describe() calls should always define the subject in a way that it's understandable within the project.

describe( 'Editor', () => {
    describe( 'constructor', () => {
    } );

    ...
} );
scofalik commented 8 years ago

+1

I've noticed the same in Tree Model when I started to work on it and I have changed multiple tests, but probably some of them are still unwrapped.

szymonkups commented 8 years ago

+1

pjasiun commented 8 years ago

:+1:

Reinmar commented 8 years ago

Documented: https://github.com/ckeditor/ckeditor5-design/wiki/Code-Style#tests