Improve stability of package and tests

[jacekbogdanski]

I've fixed some tests so the whole test suite is more stable now. Also, I've corrected the implementation of getEditorNamespace because:

• in case of error, it didn't clean up the promise
• in case of empty string, it directly threw an error, which is not consistent with promise-based functions (you shouldn't have to decorate promised function with try/catch block)

The tests are still failing because of https://github.com/ckeditor/ckeditor4-angular/blob/dc423da5ecc473a834dd6b31fd6b53918f30fe37/src/ckeditor/ckeditor.component.spec.ts#L122-L170 test suite. However, those tests should no longer validate this case, as we already fixed upstream CKEditor 4 issue which required using divarea plugin for integrations. I will extract the issue as a separate ticket, as we should clean up the production code also.

Proposed changelog entry

* [#128](https://github.com/ckeditor/ckeditor4-angular/issues/128): Improve the stability of `getEditorNamespace()` method.

Closes #121 Closes #129. Closes #128.

[f1ames]

I suppose it closes #93 too?

For anyone merging it, I think we should add changelog entry too, since @jacekbogdanski improved (getEditorNamespace) which is a part of he production code. And for the backlog entry we need an issue :joy: :see_no_evil: :chains:

[jacekbogdanski]

I suppose it closes #93 too?

Yes, of course, looks like I confused ticket numbers.

For anyone merging it, I think we should add changelog entry too, since @jacekbogdanski improved (getEditorNamespace) which is a part of he production code. And for the backlog entry we need an issue joy see_no_evil chains

I wasn't sure about that because this change doesn't seem to matter from user perspective. But indeed in case if someone changes CDN URL into an empty string she/he will get console error instead of breaking the whole angular app. Changelog entry won't hurt.

I've extracted ticket for updating failing tests / divarea leftovers: https://github.com/ckeditor/ckeditor4-angular/issues/127

[jacekbogdanski]

Actually, I checked it right now and I see that a user gets exactly the same error message after proposed changes and the app behaves the same. I'm for skipping changelog entry and separate issue for that.

[f1ames]

Actually, I checked it right now and I see that a user gets exactly the same error message after proposed changes and the app behaves the same. I'm for skipping changelog entry and separate issue for that.

Good point, I was thinking more about the improvement itself and that it can solve some real-live edge cases (since now we clear and reject promise correctly, which could cause some issues earlier), so it might be useful info for developers anyway (and additional changelog entry indicates that we are working actively on the integration which is good sign for developers too). WDYT?

[Dumluregn]

Created #128 to provide a changelog entry here.

[jacekbogdanski]

PR lacks the change of karma.conf.js removing the unrandomising snippet:

I'm not sure about that. Test still sometimes fails, probably also because of https://github.com/ckeditor/ckeditor4-angular/issues/127

I propose to do that once we fix #127 and we are sure that CI will be green with random order.

[Dumluregn]

There was something wrong with the commit history, so I re-rebased this branch onto latest master.

[jacekbogdanski]

Well, the console.warn tests are failing because you fixed them smile So CI will be red for now no matter if you run tests in order or randomly. So are we good with adding this Karma config? Or you meant something different?

Yeah, they are failing despite random order, so let's make it random by default.

[jacekbogdanski]

We come to the conclusion that it takes too much time to fix running tests in random order. Instead, we will just focus on improving overall asynchronous test correctness proposed in this PR. I've changed Closes, so it will close correct issues. I've also reverted changes to karma config.