-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
fix: move test helpers from samples into core #5969
Merged
Merged
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
99af064
fix: move core test helpers into new directory
BeksOmega eff84af
fix: add test helpers to core and convert to goog modules
BeksOmega e276745
fix: change tests to use local helpers
BeksOmega fb57f18
fix: change local tests to use chai asserts
BeksOmega 3a655c5
fix: skip field tests in serializer b/c test blocks are unavailable
BeksOmega 0ded768
fix: rename some helper files
BeksOmega 08cddc8
fix: rename some helper modules
BeksOmega 6d47b07
fix: split block helpers into code gen and serialization
BeksOmega 8a78f68
fix: split block defs into new helper file
BeksOmega e084e91
fix: split warning helpers into new file
BeksOmega ccfd888
fix: split user input helpers into new file
BeksOmega e1ad627
fix: split event helpers into a new file
BeksOmega 0766370
fix: split variable helper into new file
BeksOmega b332bdf
fix: move remaining test helpers to new setup-teardown file
BeksOmega e2dd397
fix: rename setup and teardown module
BeksOmega a72953c
fix: cleanup from rebase
BeksOmega acb3e3b
fix: undo accidental rename
BeksOmega 39db79b
fix: lint?
BeksOmega 9c13a36
fix: bad toolbox definitions namespace
BeksOmega 0e39153
fix: fixup warning helpers
BeksOmega 1f8eb8f
fix: remove inclusion of dev-tools in mocha tests
BeksOmega 220d7bb
move to modules, but break mocha
BeksOmega 4dac25a
fix: run mocha as a module
BeksOmega 050956d
fix: lint
BeksOmega File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,240 @@ | ||
/** | ||
* @license | ||
* Copyright 2020 Google LLC | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
goog.module('Blockly.test.blockHelpers'); | ||
|
||
const {runTestCases, runTestSuites, TestCase, TestSuite} = goog.require('Blockly.test.commonHelpers'); | ||
|
||
|
||
/** | ||
* Code generation test case configuration. | ||
* @implements {TestCase} | ||
* @record | ||
*/ | ||
class CodeGenerationTestCase { | ||
/** | ||
* Class for a code generation test case. | ||
*/ | ||
constructor() { | ||
/** | ||
* @type {string} The expected code. | ||
*/ | ||
this.expectedCode; | ||
/** | ||
* @type {boolean|undefined} Whether to use workspaceToCode instead of | ||
* blockToCode for test. | ||
*/ | ||
this.useWorkspaceToCode; | ||
/** | ||
* @type {number|undefined} The expected inner order. | ||
*/ | ||
this.expectedInnerOrder; | ||
} | ||
|
||
/** | ||
* Creates the block to use for this test case. | ||
* @param {!Blockly.Workspace} workspace The workspace context for this | ||
* test. | ||
* @return {!Blockly.Block} The block to use for the test case. | ||
*/ | ||
createBlock(workspace) {} | ||
} | ||
exports.CodeGenerationTestCase = CodeGenerationTestCase; | ||
|
||
/** | ||
* Code generation test suite. | ||
* @extends {TestSuite<CodeGenerationTestCase, CodeGenerationTestSuite>} | ||
* @record | ||
*/ | ||
class CodeGenerationTestSuite { | ||
/** | ||
* Class for a code generation test suite. | ||
*/ | ||
constructor() { | ||
/** | ||
* @type {!Blockly.Generator} The generator to use for running test cases. | ||
*/ | ||
this.generator; | ||
} | ||
} | ||
exports.CodeGenerationTestSuite = CodeGenerationTestSuite; | ||
|
||
/** | ||
* Serialization test case. | ||
* @implements {TestCase} | ||
* @record | ||
*/ | ||
class SerializationTestCase { | ||
/** | ||
* Class for a block serialization test case. | ||
*/ | ||
constructor() { | ||
/** | ||
* @type {string} The block xml to use for test. Do not provide if json is | ||
* provided. | ||
*/ | ||
this.xml; | ||
/** | ||
* @type {string|undefined} The expected xml after round trip. Provided if | ||
* it different from xml that was passed in. | ||
*/ | ||
this.expectedXml; | ||
/** | ||
* @type {string} The block json to use for test. Do not provide if xml is | ||
* provided. | ||
*/ | ||
this.json; | ||
/** | ||
* @type {string|undefined} The expected json after round trip. Provided if | ||
* it is different from json that was passed in. | ||
*/ | ||
this.expectedJson; | ||
} | ||
/** | ||
* Asserts that the block created from xml has the expected structure. | ||
* @param {!Blockly.Block} block The block to check. | ||
*/ | ||
assertBlockStructure(block) {} | ||
} | ||
exports.SerializationTestCase = SerializationTestCase; | ||
|
||
/** | ||
* Returns mocha test callback for code generation based on provided | ||
* generator. | ||
* @param {!Blockly.Generator} generator The generator to use in test. | ||
* @return {function(!CodeGenerationTestCase):!Function} Function that | ||
* returns mocha test callback based on test case. | ||
* @private | ||
*/ | ||
const createCodeGenerationTestFn_ = (generator) => { | ||
return (testCase) => { | ||
return function() { | ||
const block = testCase.createBlock(this.workspace); | ||
let code; | ||
let innerOrder; | ||
if (testCase.useWorkspaceToCode) { | ||
code = generator.workspaceToCode(this.workspace); | ||
} else { | ||
generator.init(this.workspace); | ||
code = generator.blockToCode(block); | ||
if (Array.isArray(code)) { | ||
innerOrder = code[1]; | ||
code = code[0]; | ||
} | ||
} | ||
const assertFunc = (typeof testCase.expectedCode === 'string') ? | ||
assert.equal : assert.match; | ||
assertFunc(code, testCase.expectedCode); | ||
if (!testCase.useWorkspaceToCode && | ||
testCase.expectedInnerOrder !== undefined) { | ||
assert.equal(innerOrder, testCase.expectedInnerOrder); | ||
} | ||
}; | ||
}; | ||
}; | ||
|
||
/** | ||
* Runs blockToCode test suites. | ||
* @param {!Array<!CodeGenerationTestSuite>} testSuites The test suites to run. | ||
*/ | ||
const runCodeGenerationTestSuites = (testSuites) => { | ||
/** | ||
* Creates function used to generate mocha test callback. | ||
* @param {!CodeGenerationTestSuite} suiteInfo The test suite information. | ||
* @return {function(!CodeGenerationTestCase):!Function} Function that | ||
* creates mocha test callback. | ||
*/ | ||
const createTestFn = (suiteInfo) => { | ||
return createCodeGenerationTestFn_(suiteInfo.generator); | ||
}; | ||
|
||
runTestSuites(testSuites, createTestFn); | ||
}; | ||
exports.runCodeGenerationTestSuites = runCodeGenerationTestSuites; | ||
|
||
/** | ||
* Runs serialization test suite. | ||
* @param {!Array<!SerializationTestCase>} testCases The test cases to run. | ||
*/ | ||
const runSerializationTestSuite = (testCases) => { | ||
/** | ||
* Creates test callback for xmlToBlock test. | ||
* @param {!SerializationTestCase} testCase The test case information. | ||
* @return {!Function} The test callback. | ||
*/ | ||
const createSerializedDataToBlockTestCallback = (testCase) => { | ||
return function() { | ||
let block; | ||
if (testCase.json) { | ||
block = Blockly.serialization.blocks.append( | ||
testCase.json, this.workspace); | ||
} else { | ||
block = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( | ||
testCase.xml), this.workspace); | ||
} | ||
testCase.assertBlockStructure(block); | ||
}; | ||
}; | ||
/** | ||
* Creates test callback for xml round trip test. | ||
* @param {!SerializationTestCase} testCase The test case information. | ||
* @return {!Function} The test callback. | ||
*/ | ||
const createRoundTripTestCallback = (testCase) => { | ||
return function() { | ||
if (testCase.json) { | ||
const block = Blockly.serialization.blocks.append( | ||
testCase.json, this.workspace); | ||
const generatedJson = Blockly.serialization.blocks.save(block); | ||
const expectedJson = testCase.expectedJson || testCase.json; | ||
assert.deepEqual(generatedJson, expectedJson); | ||
} else { | ||
const block = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( | ||
testCase.xml), this.workspace); | ||
const generatedXml = | ||
Blockly.Xml.domToPrettyText( | ||
Blockly.Xml.blockToDom(block)); | ||
const expectedXml = testCase.expectedXml || testCase.xml; | ||
assert.equal(generatedXml, expectedXml); | ||
} | ||
}; | ||
}; | ||
suite('Serialization', function() { | ||
suite('xmlToBlock', function() { | ||
runTestCases(testCases, createSerializedDataToBlockTestCallback); | ||
}); | ||
suite('xml round-trip', function() { | ||
setup(function() { | ||
// The genUid is undergoing change as part of the 2021Q3 | ||
// goog.module migration: | ||
// | ||
// - It is being moved from Blockly.utils to | ||
// Blockly.utils.idGenerator (which itself is being renamed | ||
// from IdGenerator). | ||
// - For compatibility with changes to the module system (from | ||
// goog.provide to goog.module and in future to ES modules), | ||
// .genUid is now a wrapper around .TEST_ONLY.genUid, which | ||
// can be safely stubbed by sinon or other similar | ||
// frameworks in a way that will continue to work. | ||
if (Blockly.utils.idGenerator && | ||
Blockly.utils.idGenerator.TEST_ONLY) { | ||
sinon.stub(Blockly.utils.idGenerator.TEST_ONLY, 'genUid') | ||
.returns('1'); | ||
} else { | ||
// Fall back to stubbing original version on Blockly.utils. | ||
sinon.stub(Blockly.utils, 'genUid').returns('1'); | ||
} | ||
}); | ||
|
||
teardown(function() { | ||
sinon.restore(); | ||
}); | ||
|
||
runTestCases(testCases, createRoundTripTestCallback); | ||
}); | ||
}); | ||
}; | ||
exports.runSerializationTestSuite = runSerializationTestSuite; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
/** | ||
* @license | ||
* Copyright 2020 Google LLC | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
goog.module('Blockly.test.commonHelpers'); | ||
|
||
/** | ||
* Test case configuration. | ||
* @record | ||
*/ | ||
class TestCase { | ||
/** | ||
* Class for a test case configuration. | ||
*/ | ||
constructor() { | ||
/** | ||
* @type {string} The title for the test case. | ||
*/ | ||
this.title; | ||
/** | ||
* @type {boolean|undefined} Whether this test case should be skipped. | ||
* Used to skip buggy test case and should have an associated bug. | ||
*/ | ||
this.skip; | ||
/** | ||
* @type {boolean|undefined} Whether this test case should be called as | ||
* only. Used for debugging. | ||
*/ | ||
this.only; | ||
} | ||
} | ||
exports.TestCase = TestCase; | ||
|
||
/** | ||
* Test suite configuration. | ||
* @record | ||
* @template {TestCase} T | ||
* @template {TestSuite} U | ||
*/ | ||
class TestSuite { | ||
/** | ||
* Class for a test suite configuration. | ||
*/ | ||
constructor() { | ||
/** | ||
* @type {string} The title for the test case. | ||
*/ | ||
this.title; | ||
/** | ||
* @type {?Array<T>} The associated test cases. | ||
*/ | ||
this.testCases; | ||
/** | ||
* @type {?Array<U>} List of nested inner test suites. | ||
*/ | ||
this.testSuites; | ||
/** | ||
* @type {boolean|undefined} Whether this test suite should be skipped. | ||
* Used to skip buggy test case and should have an associated bug. | ||
*/ | ||
this.skip; | ||
/** | ||
* @type {boolean|undefined} Whether this test suite should be called as | ||
* only. Used for debugging. | ||
*/ | ||
this.only; | ||
} | ||
} | ||
exports.TestSuite = TestSuite; | ||
|
||
/** | ||
* Runs provided test cases. | ||
* @template {TestCase} T | ||
* @param {!Array<T>} testCases The test cases to run. | ||
* @param {function(T):Function} createTestCallback Creates test | ||
* callback using given test case. | ||
*/ | ||
function runTestCases(testCases, createTestCallback) { | ||
testCases.forEach((testCase) => { | ||
let testCall = (testCase.skip ? test.skip : test); | ||
testCall = (testCase.only ? test.only : testCall); | ||
testCall(testCase.title, createTestCallback(testCase)); | ||
}); | ||
} | ||
exports.runTestCases = runTestCases; | ||
|
||
/** | ||
* Runs provided test suite. | ||
* @template {TestCase} T | ||
* @template {TestSuite<T, U>} U | ||
* @param {Array<!U>} testSuites The test suites to run. | ||
* @param {function(!U):(function(T):!Function) | ||
* } createTestCaseCallback Creates test case callback using given test | ||
* suite. | ||
*/ | ||
function runTestSuites(testSuites, createTestCaseCallback) { | ||
testSuites.forEach((testSuite) => { | ||
let suiteCall = (testSuite.skip ? suite.skip : suite); | ||
suiteCall = (testSuite.only ? suite.only : suiteCall); | ||
suiteCall(testSuite.title, function() { | ||
if (testSuite.testSuites && testSuite.testSuites.length) { | ||
runTestSuites(testSuite.testSuites, createTestCaseCallback); | ||
} | ||
if (testSuite.testCases && testSuite.testCases.length) { | ||
runTestCases(testSuite.testCases, createTestCaseCallback(testSuite)); | ||
} | ||
}); | ||
}); | ||
} | ||
exports.runTestSuites = runTestSuites; | ||
|
||
/** | ||
* Captures the strings sent to console.warn() when calling a function. | ||
* Copies from core. | ||
* @param {Function} innerFunc The function where warnings may called. | ||
* @return {Array<string>} The warning messages (only the first arguments). | ||
*/ | ||
function captureWarnings(innerFunc) { | ||
const msgs = []; | ||
const nativeConsoleWarn = console.warn; | ||
try { | ||
console.warn = function(msg) { | ||
msgs.push(msg); | ||
}; | ||
innerFunc(); | ||
} finally { | ||
console.warn = nativeConsoleWarn; | ||
} | ||
return msgs; | ||
} | ||
exports.captureWarnings = captureWarnings; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Throughout, was the conversion to goog modules necessary? It appears these were already ES modules in dev-tools, so moving backwards seems not ideal.
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.
Honestly, not sure. Before it seems we were just accessing these via
testHelpers
which was a global thing. I could try to move back to that. Or is there a better way to access ES modules from inside closure modules?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 think you should be able to just do goog.declareModuleId() (per https://github.com/google/closure-compiler/wiki/Migrating-from-goog.modules-to-ES6-modules), and then still be able to goog.require it from Closure modules even though it's an ES module.
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 tried, but for some reason it's "breaking" the tests. The suites are being evaluated, but the tests are not (based on console logging). You can check out my changes here.
As far as I can tell, what I did matches what you did in this commit, so I don't know what the problem could be. And I haven't been able to find anyone online having the same issue :/ Thoughts?
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.
Looks like mochajs/mocha#2816, mocha is being run before the tests are loaded. Adding
type="module"
to the script tag at the bottom of index.html that calls mocha.run fixes it, and the tests pass!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.
Thank you so much Aaron! I should have been able to find that -.- But I was looking for a closure related problem. Turns out closure wasn't the issue at all!