Skip to content
This repository has been archived by the owner on Apr 6, 2021. It is now read-only.

Use of assertions #53

Open
stavros-zavrakas opened this issue Jul 11, 2016 · 2 comments
Open

Use of assertions #53

stavros-zavrakas opened this issue Jul 11, 2016 · 2 comments

Comments

@stavros-zavrakas
Copy link

Hi there,

I am using the module but I am a bit confused how the assertions are used. As I see in the core project, you are using the assertions against the options object. For example, in the c9.core/client.js it is used like that:

assert(options.baseUrl, "Option 'baseUrl' is required") 

It is not used against imports though. For example, in the c9.vfs.standalone/standalone.js the preview handler it is imported like that:

var previewHandler = imports["preview.handler"];

What if a developer forget to add this dependency in the consume array? The previewHandler it will be undefined and it will break at runtime in this specific example. Why there is not an assertion like that:

assert(imports["preview.handler"], "preview.handler is required");

Is there something that I am missing?

@nightwing
Copy link
Contributor

Adding assertions like that for every used service would make code too verbose/unreadable.
requirement to manually update the list of dependencies in the consumes array is indeed problem, we should either make a eslint rule for it, or automatically infer list of dependencies using toString similar to what require.js does, but in the latter case performance overhead is significant for a large project like cloud9

@stavros-zavrakas
Copy link
Author

We start adding assertions and finally the code became too verbose. Finally we god a decision to check the consumes array into the unit tests. The array is exported and we can compare the result with the definition in the unit test. If someone for a reason changes the dependencies (adding or removing a dependency) the unit test will break.

By the way, I saw that you are extending the chai expect library with the setupArchitectTest function (https://github.com/c9/core/blob/b24bf63f9f5a43a9647ab5cd20bee4c3e14d55fa/plugins/c9.vfs.standalone/www/test.js#L543).

Can this abstraction be helpful to check the consume dependencies or in general testing an architect application?

Many thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants