-
Notifications
You must be signed in to change notification settings - Fork 1
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
Added tests and refactored code (Part 1) #3
Changes from all commits
dfe6641
efa1393
d1d3cdd
c9ab055
e1beaac
23c666a
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 |
---|---|---|
|
@@ -17,12 +17,12 @@ function updateDependencies(packageJson) { | |
'ember-cli-htmlbars', | ||
]; | ||
|
||
const packagesToInstall = ['@embroider/addon-shim']; | ||
|
||
packagesToDelete.forEach((packageName) => { | ||
dependencies.delete(packageName); | ||
}); | ||
|
||
const packagesToInstall = ['@embroider/addon-shim']; | ||
|
||
packagesToInstall.forEach((packageName) => { | ||
const version = decideVersion(packageName, dependencies); | ||
|
||
|
@@ -37,12 +37,17 @@ function updateDevDependencies(packageJson, options) { | |
|
||
const devDependencies = convertToMap(packageJson['devDependencies']); | ||
|
||
const packagesToDelete = [ | ||
'@embroider/macros', | ||
'ember-auto-import', | ||
'ember-cli-babel', | ||
'ember-cli-htmlbars', | ||
]; | ||
/* | ||
For the time being, we'll take the approach of starting over and | ||
adding back the development dependencies that are required. For | ||
a more conservative approach, we could delete only the following: | ||
|
||
- @embroider/macros | ||
- ember-auto-import | ||
- ember-cli-babel | ||
- ember-cli-htmlbars | ||
*/ | ||
devDependencies.clear(); | ||
|
||
const packagesToInstall = packages.addon.hasTypeScript | ||
? [ | ||
|
@@ -65,13 +70,6 @@ function updateDevDependencies(packageJson, options) { | |
'rollup-plugin-copy', | ||
]; | ||
|
||
// May be easier to start over and add missing dependencies | ||
devDependencies.clear(); | ||
|
||
packagesToDelete.forEach((packageName) => { | ||
devDependencies.delete(packageName); | ||
}); | ||
Comment on lines
-71
to
-73
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. Unused code. To be precise, a no-op because |
||
|
||
packagesToInstall.forEach((packageName) => { | ||
const version = decideVersion(packageName, devDependencies); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,7 +37,10 @@ function moveDependenciesToDevDependencies(packageJson, options) { | |
function updateDependencies(packageJson) { | ||
const dependencies = convertToMap(packageJson['dependencies']); | ||
|
||
// May be easier to start over and add missing dependencies | ||
/* | ||
For the time being, we'll take the approach of starting over and | ||
adding back the dependencies that are required. | ||
*/ | ||
dependencies.clear(); | ||
|
||
packageJson['dependencies'] = convertToObject(dependencies); | ||
|
@@ -58,9 +61,9 @@ function updateOtherFields(packageJson, options) { | |
|
||
delete packageJson['ember-addon']; | ||
|
||
packageJson['keywords'] = packageJson['keywords'].filter((keyword) => { | ||
return keyword !== 'ember-addon'; | ||
}); | ||
packageJson['keywords'] = (packageJson['keywords'] ?? []).filter( | ||
(keyword) => keyword !== 'ember-addon' | ||
); | ||
Comment on lines
+64
to
+66
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 |
||
|
||
packageJson['name'] = packages.testApp.name; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
import { analyzeAddon } from '../../../../../src/migration/ember-addon/steps/analyze-addon.js'; | ||
import { assert, loadFixture, test } from '../../../../test-helpers.js'; | ||
|
||
test('migration | ember-addon | steps | analyze-addon > base case', function () { | ||
const options = { | ||
addonLocation: undefined, | ||
projectRoot: 'tmp/ember-container-query-typescript', | ||
testAppLocation: undefined, | ||
testAppName: undefined, | ||
}; | ||
|
||
const inputProject = { | ||
addon: { | ||
components: { | ||
'container-query.hbs': '', | ||
'container-query.ts': '', | ||
}, | ||
helpers: { | ||
'aspect-ratio.ts': '', | ||
'height.ts': '', | ||
'width.ts': '', | ||
}, | ||
modifiers: { | ||
'container-query.ts': '', | ||
}, | ||
styles: { | ||
'container-query.css': '', | ||
}, | ||
'.gitkeep': '', | ||
'index.ts': '', | ||
'template-registry.ts': '', | ||
}, | ||
app: { | ||
components: { | ||
'container-query.js': '', | ||
}, | ||
helpers: { | ||
'aspect-ratio.js': '', | ||
'height.js': '', | ||
'width.js': '', | ||
}, | ||
modifiers: { | ||
'container-query.js': '', | ||
}, | ||
'.gitkeep': '', | ||
}, | ||
}; | ||
|
||
const expectedValue = { | ||
appReexports: [ | ||
'components/container-query.js', | ||
'helpers/aspect-ratio.js', | ||
'helpers/height.js', | ||
'helpers/width.js', | ||
'modifiers/container-query.js', | ||
], | ||
publicEntrypoints: [ | ||
'components/container-query.js', | ||
'helpers/aspect-ratio.js', | ||
'helpers/height.js', | ||
'helpers/width.js', | ||
'index.js', | ||
'modifiers/container-query.js', | ||
'template-registry.js', | ||
], | ||
}; | ||
|
||
loadFixture(inputProject, options); | ||
|
||
// Run the step | ||
const augmentedOptions = { | ||
projectRoot: 'tmp/ember-container-query-typescript', | ||
}; | ||
|
||
assert.deepEqual(analyzeAddon(augmentedOptions), expectedValue); | ||
|
||
// Check idempotence | ||
assert.deepEqual(analyzeAddon(augmentedOptions), expectedValue); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import { analyzeAddon } from '../../../../../src/migration/ember-addon/steps/analyze-addon.js'; | ||
import { assert, loadFixture, test } from '../../../../test-helpers.js'; | ||
|
||
test('migration | ember-addon | steps | analyze-addon > edge case (folders are missing)', function () { | ||
const options = { | ||
addonLocation: undefined, | ||
projectRoot: 'tmp/new-v1-addon-javascript', | ||
testAppLocation: undefined, | ||
testAppName: undefined, | ||
}; | ||
|
||
const inputProject = {}; | ||
|
||
const expectedValue = { | ||
appReexports: [], | ||
publicEntrypoints: [], | ||
}; | ||
|
||
loadFixture(inputProject, options); | ||
|
||
// Run the step | ||
const augmentedOptions = { | ||
projectRoot: 'tmp/new-v1-addon-javascript', | ||
}; | ||
|
||
assert.deepEqual(analyzeAddon(augmentedOptions), expectedValue); | ||
|
||
// Check idempotence | ||
assert.deepEqual(analyzeAddon(augmentedOptions), expectedValue); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import { augmentOptions } from '../../../../../src/migration/ember-addon/steps/augment-options.js'; | ||
import { assert, loadFixture, test } from '../../../../test-helpers.js'; | ||
|
||
test('migration | ember-addon | steps | augment-options > error handling (corrupt package.json)', function () { | ||
test('migration | ember-addon | steps | augment-options > error handling (package.json is not a valid JSON)', function () { | ||
const options = { | ||
addonLocation: undefined, | ||
projectRoot: 'tmp/new-v1-addon-javascript', | ||
|
@@ -10,8 +10,8 @@ test('migration | ember-addon | steps | augment-options > error handling (corrup | |
}; | ||
|
||
const inputProject = { | ||
'package.json': '{\n name:}', | ||
'yarn.lock': 'some code for yarn.lock', | ||
'package.json': '{\n "name": }', | ||
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. I meant to test what would happen when the value is missing. Instead, I had incorrectly tested the case of a JSON key having an incorrect format (because the double quotations are missing). |
||
'yarn.lock': '', | ||
}; | ||
|
||
loadFixture(inputProject, options); | ||
|
@@ -23,7 +23,7 @@ test('migration | ember-addon | steps | augment-options > error handling (corrup | |
(error) => { | ||
assert.strictEqual( | ||
error.message, | ||
'ERROR: package.json is missing or is not a valid JSON. (Unexpected token n in JSON at position 4)\n' | ||
'ERROR: package.json is missing or is not a valid JSON. (Unexpected token } in JSON at position 12)\n' | ||
); | ||
|
||
return true; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import { augmentOptions } from '../../../../../src/migration/ember-addon/steps/augment-options.js'; | ||
import { assert, loadFixture, test } from '../../../../test-helpers.js'; | ||
|
||
test('migration | ember-addon | steps | augment-options > error handling (package version is missing)', function () { | ||
const options = { | ||
addonLocation: undefined, | ||
projectRoot: 'tmp/new-v1-addon-javascript', | ||
testAppLocation: undefined, | ||
testAppName: undefined, | ||
}; | ||
|
||
const inputProject = { | ||
'package.json': JSON.stringify( | ||
{ | ||
name: 'new-v1-addon', | ||
}, | ||
null, | ||
2 | ||
), | ||
'yarn.lock': '', | ||
}; | ||
|
||
loadFixture(inputProject, options); | ||
|
||
assert.throws( | ||
() => { | ||
augmentOptions(options); | ||
}, | ||
(error) => { | ||
assert.strictEqual( | ||
error.message, | ||
'ERROR: In package.json, the package version is missing.' | ||
); | ||
|
||
return true; | ||
} | ||
); | ||
}); |
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.
According to
npm
, we can expect that a validpackage.json
will always have thename
andversion
fields.https://docs.npmjs.com/creating-a-package-json-file#required-name-and-version-fields