-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
refactor(tests): convert to TypeScript #508
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
"^es/", | ||
"^helpers/", | ||
"^modules/", | ||
"^test/", | ||
"^h.d.ts", | ||
"^h.js", | ||
"^h.js.map", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ | |
/es/modules/style.d.ts | ||
/es/modules/style.js | ||
/es/modules/style.js.map | ||
/es/test/ | ||
/es/h.d.ts | ||
/es/h.js | ||
/es/h.js.map | ||
|
@@ -92,6 +93,7 @@ | |
/modules/style.d.ts | ||
/modules/style.js | ||
/modules/style.js.map | ||
/test/ | ||
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. Same as above. This is from the CommonJS build. |
||
/h.d.ts | ||
/h.js | ||
/h.js.map | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,7 @@ module.exports = function(config) { | |
basePath: '.', | ||
frameworks: ['mocha', 'karma-typescript'], | ||
// list of files / patterns to load in the browser | ||
files: [{pattern: 'src/**/*.ts'}, {pattern: 'test/**/*'}], | ||
files: [{pattern: 'src/**/*.ts'}], | ||
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. The tests are moved to |
||
plugins: [ | ||
'karma-mocha', | ||
'karma-chrome-launcher', | ||
|
@@ -29,8 +29,7 @@ module.exports = function(config) { | |
], | ||
hostname: ci ? ip : 'localhost', | ||
preprocessors: { | ||
'src/**/*.ts': ['karma-typescript'], | ||
'test/**/*.{js,ts,tsx}': ['karma-typescript'], | ||
'src/**/*.{ts,tsx}': ['karma-typescript'] | ||
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. Only one preprocessor definition, because everything is under |
||
}, | ||
browserStack: { | ||
name: 'Snabbdom', | ||
|
@@ -43,16 +42,12 @@ module.exports = function(config) { | |
customLaunchers: browserstack, | ||
karmaTypescriptConfig: { | ||
coverageOptions: { | ||
exclude: /test\//, | ||
}, | ||
compilerOptions: { | ||
allowJs: true, | ||
declaration: false | ||
Comment on lines
-49
to
-50
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 doesn't seem that these two serve any purpose. |
||
exclude: /^src{\/|\\}test{\/|\\}/, | ||
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. Fixes #509. |
||
}, | ||
tsconfig: './tsconfig.json', | ||
include: { | ||
mode: 'merge', | ||
values: ['test/**/*'], | ||
values: ['src/test/**/*'], | ||
}, | ||
}, | ||
reporters: ['dots', 'karma-typescript', 'BrowserStack'], | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
}, | ||
"devDependencies": { | ||
"@types/assert": "^1.4.3", | ||
"@types/lodash.shuffle": "^4.2.6", | ||
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. See below. |
||
"@types/mocha": "^5.2.7", | ||
"@typescript-eslint/eslint-plugin": "^2.7.0", | ||
"benchmark": "^2.1.4", | ||
|
@@ -22,7 +23,6 @@ | |
"eslint-plugin-node": "^10.0.0", | ||
"eslint-plugin-promise": "^4.2.1", | ||
"eslint-plugin-standard": "^4.0.1", | ||
"fake-raf": "1.0.1", | ||
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. Replaced this with real calls to |
||
"gulp": "^3.9.1", | ||
"gulp-clean": "^0.3.2", | ||
"gulp-rename": "^1.2.2", | ||
|
@@ -35,7 +35,7 @@ | |
"karma-firefox-launcher": "^1.2.0", | ||
"karma-mocha": "^1.3.0", | ||
"karma-typescript": "^3.0.13", | ||
"knuth-shuffle": "^1.0.1", | ||
"lodash.shuffle": "^4.2.0", | ||
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. Because I couldn't find TypeScript types for knuth-shuffle. It's only used in a test so it doesn't seem to matter what is used. |
||
"mocha": "^5.2.0", | ||
"npm-run-all": "^4.1.5", | ||
"remark-cli": "^7.0.1", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,13 @@ | ||
var assert = require('assert'); | ||
var snabbdom = require('../snabbdom'); | ||
import assert from 'assert' | ||
import { init } from '../snabbdom' | ||
import { RemoveHook } from '../hooks'; | ||
|
||
var patch = snabbdom.init([]); | ||
var attachTo = require('../helpers/attachto').default; | ||
var h = require('../h').default; | ||
var patch = init([]); | ||
import attachTo from '../helpers/attachto' | ||
import h from '../h' | ||
|
||
describe('attachTo', function() { | ||
var elm, vnode0; | ||
var elm: any, vnode0: any; | ||
Comment on lines
-9
to
+10
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. This pattern of reusing these two variables is in all test modules. These definitely should be refactored, I feel. Probably removed and replaced with variables/constants in each test. But beyond the scope of this PR. 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. Open a new issue? |
||
beforeEach(function() { | ||
elm = document.createElement('div'); | ||
vnode0 = elm; | ||
|
@@ -76,9 +77,10 @@ describe('attachTo', function() { | |
assert.equal(elm.children.length, 1); | ||
}); | ||
it('remove hook receives real element', function() { | ||
function rm(vnode, cb) { | ||
assert.equal(vnode.elm.tagName, 'DIV'); | ||
assert.equal(vnode.elm.innerHTML, 'First text'); | ||
const rm: RemoveHook = (vnode, cb) => { | ||
const elm = vnode.elm as HTMLDivElement; | ||
assert.equal(elm.tagName, 'DIV'); | ||
assert.equal(elm.innerHTML, 'First text'); | ||
cb(); | ||
} | ||
var vnode1 = h('div', [ | ||
|
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.
We have new build artifacts. The tests. They should be ignored along with the rest of the build artifacts. This is from the ES modules build.