Skip to content
This repository was archived by the owner on Dec 10, 2019. It is now read-only.

Commit ee6fcfc

Browse files
committed
fix for out-of-order pseudopattern bug -- isPatternFile() should return
true for a pseudopattern JSON file and it wasn't. In fact, its unit test was lying to us. Lies! Also: implement a proper isPseudoPatternJSON() function in pattern_engines to centralize that logic and write a unit test for it; move isPatternFile unit test to the right file
1 parent fd3c4b5 commit ee6fcfc

File tree

4 files changed

+66
-34
lines changed

4 files changed

+66
-34
lines changed

builder/pattern_assembler.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@
8989
var currentPattern = new of.oPattern(file, subdir, filename);
9090

9191
//if file is named in the syntax for variants
92-
if(ext === '.json' && filename.indexOf('~') > -1){
92+
if(patternEngines.isPseudoPatternJSON(filename)){
9393
//add current pattern to patternlab object with minimal data
9494
//processPatternRecursive() will run find_pseudopatterns() to fill out
9595
//the object in the next diveSync

builder/pattern_engines/pattern_engines.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,13 @@
6666
return (supportedExtensions.lastIndexOf(fileExtension) !== -1);
6767
},
6868

69+
// given a filename, return a boolean: whether or not the filename indicates
70+
// that the file is pseudopattern JSON
71+
isPseudoPatternJSON: function (filename) {
72+
var extension = path.extname(filename);
73+
return (extension === '.json' && filename.indexOf('~') > -1);
74+
},
75+
6976
// takes a filename string, not a full path; a basename (plus extension)
7077
// ignore _underscored patterns, dotfiles, and anything not recognized by a
7178
// loaded pattern engine. Pseudo-pattern .json files ARE considered to be
@@ -75,13 +82,14 @@
7582
var extension = path.extname(filename);
7683
if(filename.charAt(0) === '.' ||
7784
filename.charAt(0) === '_' ||
78-
(extension === '.json' && filename.indexOf('~') === -1)) {
85+
(extension === '.json' && !PatternEngines.isPseudoPatternJSON(filename))) {
7986
return false;
8087
}
8188

8289
// not a hidden pattern, let's dig deeper
8390
var supportedPatternFileExtensions = PatternEngines.getSupportedFileExtensions();
84-
return (supportedPatternFileExtensions.lastIndexOf(extension) !== -1);
91+
return (supportedPatternFileExtensions.lastIndexOf(extension) !== -1 ||
92+
PatternEngines.isPseudoPatternJSON(filename));
8593
}
8694
});
8795

test/pattern_assembler_tests.js

Lines changed: 4 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
var pa = require('../builder/pattern_assembler');
55
var of = require('../builder/object_factory');
6-
var patternEngines = require('../builder/pattern_engines/pattern_engines');
76

87
exports['pattern_assembler'] = {
98
'find_pattern_partials finds partials' : function(test){
@@ -31,6 +30,9 @@
3130
"{{> molecules-single-comment(description: 'A life isn\\'t like a garden. Perfect moments can be had, but not preserved, except in memory.') }}" +
3231
'{{> molecules-single-comment(description: "A life is like a \\"garden\\". Perfect moments can be had, but not preserved, except in memory.") }}' +
3332
"{{> molecules-single-comment:foo }}" +
33+
// questionable: is a partial call with a file extension really
34+
// permitted? seems like no, based on
35+
// http://patternlab.io/docs/pattern-including.html
3436
"{{> 01-molecules/06-components/03-comment-header.mustache }}" +
3537
"{{> 01-molecules/06-components/02-single-comment.mustache(description: 'A life is like a garden. Perfect moments can be had, but not preserved, except in memory.') }}" +
3638
"{{> molecules-single-comment:foo }}" +
@@ -354,35 +356,7 @@
354356
//test that 00-foo.mustache included partial 01-bar.mustache
355357
test.equals(fooExtended, 'bar');
356358

357-
test.done();
358-
},
359-
360-
'isPatternFile correctly identifies pattern files and rejects non-pattern files': function(test){
361-
var pattern_assembler = new pa();
362-
363-
// each test case
364-
var filenames = {
365-
'00-comment-thread.mustache': true,
366-
'00-comment-thread.fakeextthatdoesntexist': false,
367-
'00-comment-thread': false,
368-
'_00-comment-thread.mustache': false,
369-
'.00-comment-thread.mustache': false,
370-
'00-comment-thread.json': false,
371-
'00-homepage~emergency.json': false
372-
};
373-
// expect one test per test case
374-
test.expect(Object.keys(filenames).length);
375-
376-
// loop over each test case and test it
377-
Object.keys(filenames).forEach(function (filename) {
378-
var expectedResult = filenames[filename],
379-
actualResult = patternEngines.isPatternFile(filename),
380-
testMessage = 'isPatternFile should return ' + expectedResult + ' for ' + filename;
381-
test.strictEqual(actualResult, expectedResult, testMessage);
382-
});
383-
384-
// done
385359
test.done();
386360
}
387361
};
388-
}());
362+
})();

test/pattern_engines_tests.js

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,57 @@
3939
var engine = patternEngines.getEngineForPattern(mustacheTestPseudoPattern);
4040
test.equals(engine, patternEngines.mustache);
4141
test.done();
42-
}
42+
},
43+
'isPseudoPatternJSON correctly identifies pseudo-pattern JSON filenames': function(test) {
44+
// each test case
45+
var filenames = {
46+
'00-homepage~emergency.json': true,
47+
'~emergency.json': true,
48+
'00-homepage~emergency.js': false,
49+
'00-homepage-emergency.js': false,
50+
'00-homepage.hbs': false,
51+
'00-homepage.json': false,
52+
'greatpic.jpg': false
53+
};
54+
// expect one test per test case
55+
test.expect(Object.keys(filenames).length);
56+
57+
// loop over each test case and test it
58+
Object.keys(filenames).forEach(function (filename) {
59+
var expectedResult = filenames[filename],
60+
actualResult = patternEngines.isPseudoPatternJSON(filename),
61+
testMessage = 'isPseudoPatternJSON should return ' + expectedResult + ' for ' + filename;
62+
test.strictEqual(actualResult, expectedResult, testMessage);
63+
});
64+
65+
// done
66+
test.done();
67+
},
68+
'isPatternFile correctly identifies pattern files and rejects non-pattern files': function(test){
69+
// each test case
70+
var filenames = {
71+
'00-comment-thread.mustache': true,
72+
'00-comment-thread.fakeextthatdoesntexist': false,
73+
'00-comment-thread': false,
74+
'_00-comment-thread.mustache': false,
75+
'.00-comment-thread.mustache': false,
76+
'00-comment-thread.json': false,
77+
'00-homepage~emergency.json': true
78+
};
79+
// expect one test per test case
80+
test.expect(Object.keys(filenames).length);
81+
82+
// loop over each test case and test it
83+
Object.keys(filenames).forEach(function (filename) {
84+
var expectedResult = filenames[filename],
85+
actualResult = patternEngines.isPatternFile(filename),
86+
testMessage = 'isPatternFile should return ' + expectedResult + ' for ' + filename;
87+
test.strictEqual(actualResult, expectedResult, testMessage);
88+
});
89+
90+
// done
91+
test.done();
92+
}
4393
};
4494

4595
// testProps() utility function: given an object, and a hash of expected

0 commit comments

Comments
 (0)