Skip to content

Fix importRoot #69

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.

Sign up for GitHub

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

Closed
casio wants to merge 1 commit into from
Closed

Fix importRoot #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions bin/suitcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,27 @@ function requireOrParse(configFile) {
return readFn(configPath);
}

var input = program.args[0] ? resolve(program.args[0]) : null;
var output = program.args[1] ? resolve(program.args[1]) : null;
var config = program.config ? requireOrParse(program.config) : {};
var verbose = program.verbose;
var regen = program.watch && input && output;
var output = program.args[1] ? resolve(program.args[1]) : null;

/**
* Exists?
* Input file.
*/

if (input && !exists(input)) logger.fatal('not found', input);
var input = program.args[0] || null;
var importRoot = program.importRoot || config.root;

if (input) {
var resolvedInput = importRoot ? resolve(importRoot, input) : resolve(input);
if (!exists(resolvedInput)) {
logger.fatal('not found', resolvedInput);
} else {
input = resolvedInput;
}
}

var regen = program.watch && input && output;

/**
* Run.
Expand Down
29 changes: 14 additions & 15 deletions test/cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,26 +48,16 @@ describe('cli', function() {
});
});

it('should allow configurable import root', function(done) {
exec('node bin/suitcss -i test/fixtures -c test/config/noautoprefixer.js test/fixtures/import.css test/fixtures/cli/output.css', function(err) {
if (err) return done(err);
var res = util.read('fixtures/cli/output');
var expected = util.read('fixtures/component.out');
expect(res).to.equal(expected);
done();
});
});

it('should output stylelint warnings', function(done) {
Copy link
Member

@giuseppeg giuseppeg Dec 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are testing other feature, would you mind creating a suite only to test the importRoot thing instead of mixing tests up?

Copy link
Contributor Author

@casio casio Dec 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, test/cli.js currently is really more of an integration test suite, no?
so imho its ok to test all i/o stuff there.
it would probably be cleaner to eg remove the -i options out of unrelated cases, though

Copy link
Member

@giuseppeg giuseppeg Dec 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean a nested describe in test/cli.js with the importRoot relevant tests.
The point is testing imports indirectly i.e. by tweaking other tests to cover some use cases it is not optimal.

exec('node bin/suitcss -i test/fixtures -c test/config/noautoprefixer.js test/fixtures/stylelint-import.css test/fixtures/cli/output.css -l', function(err, stdout) {
exec('node bin/suitcss -i test/fixtures -c test/config/noautoprefixer.js stylelint-import.css test/fixtures/cli/output.css -l', function(err, stdout) {
if (err) return done(err);
expect(stdout).to.contain('Expected indentation of 2 spaces');
done();
});
});

it('should minify the output', function(done) {
exec('node bin/suitcss -i test/fixtures -c test/config/noautoprefixer.js test/fixtures/import.css test/fixtures/cli/output.css -m', function(err) {
exec('node bin/suitcss -i test/fixtures -c test/config/noautoprefixer.js import.css test/fixtures/cli/output.css -m', function(err) {
if (err) return done(err);
var res = util.read('fixtures/cli/output');
var expected = util.read('fixtures/minify.out');
Expand All @@ -77,7 +67,7 @@ describe('cli', function() {
});

it('should allow a config module to be passed', function(done) {
exec('node bin/suitcss -i test/fixtures -c test/config/test.js test/fixtures/config.css test/fixtures/cli/output.css', function(err) {
exec('node bin/suitcss -i test/fixtures -c test/config/test.js config.css test/fixtures/cli/output.css', function(err) {
if (err) return done(err);
var res = util.read('fixtures/cli/output');
var expected = util.read('fixtures/config.out');
Expand All @@ -87,7 +77,7 @@ describe('cli', function() {
});

it('should allow an arbitrarily named json config file to be passed', function(done) {
exec('node bin/suitcss -i test/fixtures -c test/config/test.config test/fixtures/config.css test/fixtures/cli/output.css', function(err) {
exec('node bin/suitcss -i test/fixtures -c test/config/test.config config.css test/fixtures/cli/output.css', function(err) {
if (err) return done(err);
var res = util.read('fixtures/cli/output');
var expected = util.read('fixtures/config.out');
Expand Down Expand Up @@ -117,7 +107,7 @@ describe('cli', function() {
});

it('should output an error to stderr on conformance failure when --throw-error is set', function(done) {
exec('node bin/suitcss -i test/fixtures -e test/fixtures/import-error.css test/fixtures/cli/output.css', function(err, stdout, stderr) {
exec('node bin/suitcss -i test/fixtures -e import-error.css test/fixtures/cli/output.css', function(err, stdout, stderr) {
expect(err).to.be.an('error');
expect(err.code).to.equal(1);
expect(stderr).to.contain('postcss-reporter: warnings or errors were found');
Expand All @@ -133,5 +123,14 @@ describe('cli', function() {
done();
});
});

describe('importRoot', function() {
it('should resolve input files relative to importRoot', function(done) {
exec('node bin/suitcss -i test/fixtures import.css test/fixtures/cli/output.css', function(err) {
expect(err).to.be.null;
done();
});
});
});
});