-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Major refactor of the image test scripts #656
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
Changes from 1 commit
66bf29e
98d2a4e
db381fd
592484d
caefe67
530d7c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- init test/image/assets/ - move get_image_request_option to assets/ - move image test folders init to pretest.js - factor out get image path logic in script into asset file - add + centralize glob pattern matching - add --queue option for compare pixel script - run baseline generation in queue (instead of in batch)
- Loading branch information
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
var path = require('path'); | ||
var constants = require('../../../tasks/util/constants'); | ||
|
||
var DEFAULT_FORMAT = 'png'; | ||
|
||
|
||
/** | ||
* Return paths to baseline, test-image and diff images for a given mock name. | ||
* | ||
* @param {string} mockName | ||
* @param {string} format | ||
* @return {object} | ||
* baseline | ||
* test | ||
* diff | ||
*/ | ||
module.exports = function getImagePaths(mockName, format) { | ||
format = format || DEFAULT_FORMAT; | ||
|
||
return { | ||
baseline: join(constants.pathToTestImageBaselines, mockName, format), | ||
test: join(constants.pathToTestImages, mockName, format), | ||
diff: join(constants.pathToTestImagesDiff, mockName, format) | ||
}; | ||
}; | ||
|
||
function join(basePath, mockName, format) { | ||
return path.join(basePath, mockName) + '.' + format; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
var path = require('path'); | ||
var constants = require('../../../tasks/util/constants'); | ||
|
||
var DEFAULT_URL = 'http://localhost:9010/'; | ||
var DEFAULT_FORMAT = 'png'; | ||
var DEFAULT_SCALE = 1; | ||
|
||
/** | ||
* Return the image server request options for a given mock (and specs) | ||
* | ||
* @param {object} specs | ||
* mockName : name of json mock to plot | ||
* format (optional): format of generated image | ||
* scale (optional): scale of generated image | ||
* url (optional): URL of image server | ||
*/ | ||
module.exports = function getRequestOpts(specs) { | ||
var pathToMock = path.join(constants.pathToTestImageMocks, specs.mockName) + '.json'; | ||
var figure = require(pathToMock); | ||
|
||
var body = { | ||
figure: figure, | ||
format: specs.format || DEFAULT_FORMAT, | ||
scale: specs.scale || DEFAULT_SCALE | ||
}; | ||
|
||
return { | ||
method: 'POST', | ||
url: specs.url || DEFAULT_URL, | ||
body: JSON.stringify(body) | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
var path = require('path'); | ||
var glob = require('glob'); | ||
|
||
var constants = require('../../../tasks/util/constants'); | ||
|
||
|
||
/** | ||
* Return array of mock name corresponding to input glob pattern | ||
* | ||
* @param {string} pattern | ||
* @return {array} | ||
*/ | ||
module.exports = function getMocks(pattern) { | ||
// defaults to 'all' | ||
pattern = pattern || '*'; | ||
|
||
// defaults to '.json' ext is none is provided | ||
if(path.extname(pattern) === '') pattern += '.json'; | ||
|
||
var patternFull = constants.pathToTestImageMocks + '/' + pattern; | ||
var matches = glob.sync(patternFull); | ||
|
||
// return only the mock name (not a full path, no ext) | ||
var mockNames = matches.map(function(match) { | ||
return path.basename(match).split('.')[0]; | ||
}); | ||
|
||
return mockNames; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,109 +1,156 @@ | ||
var fs = require('fs'); | ||
var path = require('path'); | ||
|
||
var constants = require('../../tasks/util/constants'); | ||
var getOptions = require('../../tasks/util/get_image_request_options'); | ||
var getMockList = require('./assets/get_mock_list'); | ||
var getRequestOpts = require('./assets/get_image_request_options'); | ||
var getImagePaths = require('./assets/get_image_paths'); | ||
|
||
// packages inside the image server docker | ||
var request = require('request'); | ||
var test = require('tape'); | ||
var request = require('request'); | ||
var gm = require('gm'); | ||
|
||
var TOLERANCE = 1e-6; // pixel comparison tolerance | ||
var BASE_TIMEOUT = 500; // base timeout time | ||
var BATCH_SIZE = 5; // size of each test 'batch' | ||
var running = 0; // number of tests currently running | ||
// pixel comparison tolerance | ||
var TOLERANCE = 1e-6; | ||
|
||
// wait time between each test batch | ||
var BATCH_WAIT = 500; | ||
|
||
// number of tests in each test batch | ||
var BATCH_SIZE = 5; | ||
|
||
// wait time between each test in test queue | ||
var QUEUE_WAIT = 10; | ||
|
||
/** | ||
* Image pixel comparison test script. | ||
* | ||
* Called by `tasks/test_image.sh in `npm run test-image`. | ||
* | ||
* CLI arguments: | ||
* | ||
* 1. 'pattern' : | ||
* glob determining which mock(s) are to be tested | ||
* 2. --queue : | ||
* if sent, the image will be run in queue instead of in batch. | ||
* Makes the test run significantly longer, but is Recommended on weak hardware. | ||
* | ||
* Examples: | ||
* | ||
* Run all tests in batch: | ||
* | ||
* npm run test-image | ||
* | ||
* Run the 'contour_nolines' test: | ||
* | ||
* npm run test-image -- contour_nolines | ||
* | ||
* Run all gl3d image test in queue: | ||
* | ||
* npm run test-image -- gl3d_* --queue | ||
*/ | ||
|
||
var pattern = process.argv[2]; | ||
var mockList = getMockList(pattern); | ||
var isInQueue = (process.argv[3] === '--queue'); | ||
|
||
if(mockList.length === 0) { | ||
throw new Error('No mocks found with pattern ' + pattern); | ||
} | ||
|
||
var touch = function(fileName) { | ||
fs.closeSync(fs.openSync(fileName, 'w')); | ||
}; | ||
mockList = mockList.filter(untestableFilter); | ||
|
||
if(mockList.length === 0) { | ||
throw new Error('All mocks found with pattern ' + pattern + ' are currently untestable'); | ||
} | ||
|
||
// make artifact folders | ||
if(!fs.existsSync(constants.pathToTestImagesDiff)) { | ||
fs.mkdirSync(constants.pathToTestImagesDiff); | ||
// main | ||
if(isInQueue) { | ||
runInQueue(mockList); | ||
} | ||
if(!fs.existsSync(constants.pathToTestImages)) { | ||
fs.mkdirSync(constants.pathToTestImages); | ||
else { | ||
runInBatch(mockList); | ||
} | ||
|
||
var userFileName = process.argv[2]; | ||
/* Test cases: | ||
* | ||
* - font-wishlist | ||
* - all gl2d | ||
* - all mapbox | ||
* | ||
* don't behave consistently from run-to-run and/or | ||
* machine-to-machine; skip over them for now. | ||
* | ||
*/ | ||
function untestableFilter(mockName) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be useful/convenient to have these as a config variable array and just check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be useful for sure. But ideally, this filter wouldn't be there in the first place. So I'd vote for the solution of least-surface-area i.e. this one ⏫ . |
||
return !( | ||
mockName === 'font-wishlist' || | ||
mockName.indexOf('gl2d_') !== -1 || | ||
mockName.indexOf('mapbox_') !== -1 | ||
); | ||
} | ||
|
||
// run the test(s) | ||
if(!userFileName) runAll(); | ||
else runSingle(userFileName); | ||
function runInBatch(mockList) { | ||
var running = 0; | ||
|
||
function runAll() { | ||
test('testing mocks', function(t) { | ||
// remove mapbox mocks if circle ci | ||
|
||
var fileNames = fs.readdirSync(constants.pathToTestImageMocks); | ||
test('testing mocks in batch', function(t) { | ||
t.plan(mockList.length); | ||
|
||
// eliminate pollutants (e.g .DS_Store) that can accumulate in the mock directory | ||
var allMocks = fileNames.filter(function(name) {return name.slice(-5) === '.json';}); | ||
for(var i = 0; i < mockList.length; i++) { | ||
run(mockList[i], t); | ||
} | ||
}); | ||
|
||
/* Test cases: | ||
* | ||
* - font-wishlist | ||
* - all gl2d | ||
* | ||
* don't behave consistently from run-to-run and/or | ||
* machine-to-machine; skip over them. | ||
* | ||
*/ | ||
var mocks = allMocks.filter(function(mock) { | ||
return !( | ||
mock === 'font-wishlist.json' || | ||
mock.indexOf('gl2d') !== -1 | ||
); | ||
}); | ||
function run(mockName, t) { | ||
if(running >= BATCH_SIZE) { | ||
setTimeout(function() { | ||
run(mockName, t); | ||
}, BATCH_WAIT); | ||
return; | ||
} | ||
running++; | ||
|
||
// skip mapbox mocks for now | ||
mocks = mocks.filter(function(mock) { | ||
return mock.indexOf('mapbox_ F438 9;) === -1; | ||
// throttle the number of tests running concurrently | ||
|
||
comparePixels(mockName, function(isEqual, mockName) { | ||
running--; | ||
t.ok(isEqual, mockName + ' should be pixel perfect'); | ||
}); | ||
} | ||
} | ||
|
||
t.plan(mocks.length); | ||
function runInQueue(mockList) { | ||
var index = 0; | ||
|
||
for(var i = 0; i < mocks.length; i++) { | ||
testMock(mocks[i], t); | ||
} | ||
test('testing mocks in queue', function(t) { | ||
t.plan(mockList.length); | ||
|
||
run(mockList[index], t); | ||
}); | ||
} | ||
|
||
function runSingle(userFileName) { | ||
test('testing single mock: ' + userFileName, function(t) { | ||
t.plan(1); | ||
testMock(userFileName, t); | ||
}); | ||
} | ||
function run(mockName, t) { | ||
comparePixels(mockName, function(isEqual, mockName) { | ||
t.ok(isEqual, mockName + ' should be pixel perfect'); | ||
|
||
function testMock(fileName, t) { | ||
// throttle the number of tests running concurrently | ||
if(running >= BATCH_SIZE) { | ||
setTimeout(function() { testMock(fileName, t); }, BASE_TIMEOUT); | ||
return; | ||
index++; | ||
if(index < mockList.length) { | ||
setTimeout(function() { | ||
run(mockList[index], t); | ||
}, QUEUE_WAIT); | ||
} | ||
}); | ||
} | ||
running++; | ||
|
||
var figure = require(path.join(constants.pathToTestImageMocks, fileName)); | ||
var bodyMock = { | ||
figure: figure, | ||
format: 'png', | ||
scale: 1 | ||
}; | ||
} | ||
|
||
var imageFileName = fileName.split('.')[0] + '.png'; | ||
var savedImagePath = path.join(constants.pathToTestImages, imageFileName); | ||
var diffPath = path.join(constants.pathToTestImagesDiff, 'diff-' + imageFileName); | ||
var savedImageStream = fs.createWriteStream(savedImagePath); | ||
var options = getOptions(bodyMock, 'http://localhost:9010/'); | ||
function comparePixels(mockName, cb) { | ||
var requestOpts = getRequestOpts({ mockName: mockName }), | ||
imagePaths = getImagePaths(mockName), | ||
saveImageStream = fs.createWriteStream(imagePaths.test); | ||
|
||
function checkImage() { | ||
running--; | ||
|
||
var options = { | ||
file: diffPath, | ||
var gmOpts = { | ||
file: imagePaths.diff, | ||
highlightColor: 'purple', | ||
tolerance: TOLERANCE | ||
}; | ||
|
@@ -125,26 +172,30 @@ function testMock(fileName, t) { | |
*/ | ||
|
||
gm.compare( | ||
savedImagePath, | ||
path.join(constants.pathToTestImageBaselines, imageFileName), | ||
options, | ||
imagePaths.test, | ||
imagePaths.baseline, | ||
gmOpts, | ||
onEqualityCheck | ||
); | ||
} | ||
|
||
function onEqualityCheck(err, isEqual) { | ||
if(err) { | ||
touch(diffPath); | ||
return console.error(err, imageFileName); | ||
touch(imagePaths.diff); | ||
return console.error(err, mockName); | ||
} | ||
if(isEqual) { | ||
fs.unlinkSync(diffPath); | ||
fs.unlinkSync(imagePaths.diff); | ||
} | ||
|
||
t.ok(isEqual, imageFileName + ' should be pixel perfect'); | ||
cb(isEqual, mockName); | ||
} | ||
|
||
request(options) | ||
.pipe(savedImageStream) | ||
request(requestOpts) | ||
.pipe(saveImageStream) | ||
.on('close', checkImage); | ||
} | ||
|
||
function touch(filePath) { | ||
fs.closeSync(fs.openSync(filePath, 'w')); | ||
} |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking about making changing these to:
to better handle machine-to-machine differences
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either env variables or a
.gitignore
'd config file works for me.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok great. I'll do this in a future PR.