Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Break apart tests #124

Open
src-code opened this issue Mar 25, 2015 · 3 comments
Open

Break apart tests #124

src-code opened this issue Mar 25, 2015 · 3 comments
Labels
enhancement package/atomizer Issues for atomizer package
Milestone

Comments

@src-code
Copy link
Contributor

Right now we have a few tests for getCss() that are a dumping ground for every unique type of class (hex values, hex + alpha, customs, child selectors, customs, etc). It'd be better if we could break this up into one test per class type, so we can be more explicit about having proper coverage for all syntax we support.

@3den
Copy link

3den commented Mar 25, 2015

Currently the getCss method is more than 200 LoC, it is hader to test one huge method that several small ones. What you guys think about simplifying this by create a class that does just css parsing:

Atomizer.prototype.getCss = function (config/*:AtomizerConfig*/, options/*:CSSOptions*/)/*:string*/ {
    return new CssParser(config).getContent(options);
}

This way we can move all that logic to CssParser and have methods there that are smaller and easier to test.

@3den
Copy link

3den commented Mar 25, 2015

@src-code @renatoi this way the API of Atomizer will stay the same but code will get easier to test and maintain.

@src-code
Copy link
Contributor Author

@3den That's outside the scope of this issue - here, we're only really interested in ensuring that we have tests for each type of syntax we support. Whether we're testing getCss() or findClassNames() or some other class is really a different issue. I'd suggest opening a new issue to start a discussion on your idea of breaking up getCss()

@renatoi renatoi added this to the 3.1.0 milestone Apr 23, 2015
redonkulus pushed a commit that referenced this issue Jul 14, 2022
redonkulus pushed a commit that referenced this issue Jul 14, 2022
@redonkulus redonkulus added the package/atomizer Issues for atomizer package label Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement package/atomizer Issues for atomizer package
Projects
None yet
Development

No branches or pull requests

4 participants