Skip to content
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

[EOSF-594] Add example controller unit tests and component integration tests #293

Merged
merged 33 commits into from
Apr 28, 2017
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
8f10f47
add facotry-guy, mockjax and acceptance test
Feb 21, 2017
7728d4c
Fix module for acceptance, play with content test some more
Feb 27, 2017
06f1d3f
Flesh out content controller tests
Feb 28, 2017
adbdbff
New component tests:
Mar 1, 2017
7313017
Adjust author-link tests
Mar 3, 2017
4f2d582
Merge branch 'develop' of http:/CenterForOpenScience/embe…
Mar 3, 2017
cd2947f
Clean up tests and remove unused code
Mar 10, 2017
227b7ba
Make sure bower has Faker
Mar 14, 2017
2091748
Update yarn with pinned ember-osf commit
hmoco Mar 20, 2017
ddaa0c9
Merge branch 'feature/tests' of http:/CenterForOpenScienc…
Mar 21, 2017
ed8659f
Update packages, add stubbing to requests in acceptance tests
Mar 21, 2017
eea027d
Merge branch 'develop' of http:/CenterForOpenScience/embe…
Mar 21, 2017
0aa4bbd
Merge branch 'feature/tests' of http:/CenterForOpenScienc…
Mar 21, 2017
514e5b6
Remove obsolete tests
Mar 21, 2017
93b096d
Update module-for-acceptance.js
hmoco Mar 22, 2017
41e49eb
skip do nothing test, start discover page setup
Mar 27, 2017
7b2f3b1
unsure on how to ensure data is available fully prior to hitting andThen
Mar 27, 2017
781ed81
Make separates and joins of copyright providers use ', ' rather than ','
Mar 27, 2017
6a20fbd
Play around some more with acceptance/runs
Mar 28, 2017
6bfd5c8
remove unused tests (acceptance) and clean up/separate other tests
Mar 29, 2017
bc75a16
Merge branch 'develop' of http:/CenterForOpenScience/embe…
hmoco Apr 6, 2017
669d71a
Merge branch 'feature/tests' of http:/CenterForOpenScienc…
hmoco Apr 6, 2017
480c3d6
Attempt to recreate yarn file
hmoco Apr 7, 2017
d4e63d3
Attempt to fix lockfile conflicts
hmoco Apr 7, 2017
a399056
Merge branch 'feature/tests' of http:/CenterForOpenScienc…
hmoco Apr 24, 2017
80c272c
Clean up packages, pin ember data facotry guy version
hmoco Apr 24, 2017
bc0e632
Merge branch 'develop' of http:/CenterForOpenScience/embe…
hmoco Apr 26, 2017
c12ce72
Add messages to test failures
hmoco Apr 26, 2017
a751d6e
Make split and joins consistent across copyrightHolders and test for it
hmoco Apr 27, 2017
c6bb595
Distinguish async behavior on discardbasics, test for copyright holde…
hmoco Apr 27, 2017
28208e4
Revert changes made to submit conroller made in c6bb5953137687cd2cd0b…
jamescdavis Apr 28, 2017
ef7d697
Merge branch 'develop' of http:/CenterForOpenScience/embe…
hmoco Apr 28, 2017
842875d
Move filedownloadUrl test to its component
hmoco Apr 28, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/content/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export default Ember.Controller.extend(Analytics, {
let text = this.get('model.license.text');
if (text) {
text = text.replace(/({{year}})/g, this.get('model.licenseRecord').year || '');
text = text.replace(/({{copyrightHolders}})/g, this.get('model.licenseRecord').copyright_holders ? this.get('model.licenseRecord').copyright_holders.join(',') : false || '');
text = text.replace(/({{copyrightHolders}})/g, this.get('model.licenseRecord').copyright_holders ? this.get('model.licenseRecord').copyright_holders.join(', ') : false || '');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though this change is minor, it'd be good to mention it in Side Effects just to bring it to attention of QA and others.

}
return text;
}),
Expand Down
8 changes: 4 additions & 4 deletions app/controllers/submit.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ export default Ember.Controller.extend(Analytics, BasicsValidations, NodeActions
let license = this.get('model.license');
return {
year: record ? record.year : null,
copyrightHolders: record && record.copyright_holders ? record.copyright_holders.join(',') : null,
copyrightHolders: record && record.copyright_holders ? record.copyright_holders.join(', ') : null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SAA about mentioning this in Side Effects.

licenseType: license || null
};
}),
Expand All @@ -313,7 +313,7 @@ export default Ember.Controller.extend(Analytics, BasicsValidations, NodeActions
if (this.get('model.licenseRecord') || this.get('model.license.content')) {
changed = changed || (this.get('model.license.name') !== this.get('basicsLicense.licenseType.name'));
changed = changed || (this.get('model.licenseRecord').year !== this.get('basicsLicense.year'));
changed = changed || ((this.get('model.licenseRecord.copyright_holders.length') ? this.get('model.licenseRecord.copyright_holders').join(',') : '') !== this.get('basicsLicense.copyrightHolders'));
changed = changed || ((this.get('model.licenseRecord.copyright_holders.length') ? this.get('model.licenseRecord.copyright_holders').join(', ') : '') !== this.get('basicsLicense.copyrightHolders'));
} else {
changed = changed || ((this.get('availableLicenses').toArray().length ? this.get('availableLicenses').toArray()[0].get('name') : null) !== this.get('basicsLicense.licenseType.name'));
let date = new Date();
Expand Down Expand Up @@ -675,7 +675,7 @@ export default Ember.Controller.extend(Analytics, BasicsValidations, NodeActions
this.set('basicsLicense', {
licenseType: license || this.get('availableLicenses').toArray()[0],
year: this.get('model.licenseRecord') ? this.get('model.licenseRecord').year : date.getUTCFullYear().toString(),
copyrightHolders: this.get('model.licenseRecord') ? this.get('model.licenseRecord').copyright_holders.join(',') : ''
copyrightHolders: this.get('model.licenseRecord') ? this.get('model.licenseRecord').copyright_holders.join(', ') : ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add a test for this.

});
});
},
Expand Down Expand Up @@ -717,7 +717,7 @@ export default Ember.Controller.extend(Analytics, BasicsValidations, NodeActions

let newCopyrightHolders = [];
if (this.get('basicsLicense.copyrightHolders') && this.get('basicsLicense.copyrightHolders').length) {
newCopyrightHolders = this.get('basicsLicense.copyrightHolders').slice().split(',');
newCopyrightHolders = this.get('basicsLicense.copyrightHolders').slice().split(', ');
}

if (this.get('abstractChanged')) node.set('description', this.get('basicsAbstract'));
Expand Down
4 changes: 3 additions & 1 deletion bower.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
"toastr": "^2.1.2",
"hint.css": "^2.3.2",
"jquery.tagsinput": "^1.3.6",
"loaders.css": "^0.1.2"
"loaders.css": "^0.1.2",
"jquery-mockjax": "~2.2.1",
"Faker": "~3.0.0"
},
"resolutions": {
"jquery": "~2.2.4"
Expand Down
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"ember-cli-code-coverage": "0.3.11",
"ember-cli-dependency-checker": "^1.2.0",
"ember-cli-dropzonejs": "0.7.0",
"ember-cli-fake-server": "0.3.2",
"ember-cli-eslint": "3.0.3",
"ember-cli-htmlbars": "^1.0.3",
"ember-cli-htmlbars-inline-precompile": "^0.3.1",
Expand All @@ -56,7 +57,9 @@
"ember-collection": "1.0.0-alpha.6",
"ember-cp-validations": "^2.9.4",
"ember-data": "^2.7.0",
"ember-data-factory-guy": "2.11.6",
"ember-export-application-global": "^1.0.5",
"ember-faker": "^1.1.1",
"ember-font-awesome": "2.1.1",
"ember-get-config": "0.0.4",
"ember-i18n": "4.3.1",
Expand All @@ -73,6 +76,8 @@
"ember-truth-helpers": "1.2.0",
"ember-welcome-page": "^1.0.1",
"eslint": "^3.18.0",
"faker": "^4.1.0",
"jscs": "^3.0.7",
"liquid-fire": "0.24.1",
"loader.js": "^4.0.1",
"pagination-pager": "2.4.2",
Expand Down
39 changes: 32 additions & 7 deletions tests/helpers/module-for-acceptance.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,47 @@ import { module } from 'qunit';
import Ember from 'ember';
import startApp from '../helpers/start-app';
import destroyApp from '../helpers/destroy-app';
import FactoryGuy, { manualSetup } from 'ember-data-factory-guy';
import config from 'ember-get-config';
import FakeServer, { stubRequest } from 'ember-cli-fake-server';

const { RSVP: { Promise } } = Ember;

export default function(name, options = {}) {
module(name, {
beforeEach() {
this.application = startApp();
this.application = startApp();
FakeServer.start();
manualSetup(this.application.__container__);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General comment for this PR: is this necessary?

From docs:

By default, you only need to call manualSetup(this.container) in unit/component tests

See also the factory guy setup for comparison in case it sparks some ideas:
https:/danielspaniel/ember-data-factory-guy/blob/master/tests/helpers/module-for-acceptance.js

const url = config.OSF.apiUrl;
const provider = FactoryGuy.build('preprint-provider');

if (options.beforeEach) {
return options.beforeEach.apply(this, arguments);
}
},
stubRequest('get', url + '/v2/users/me', (request) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks helpful!

I ran the tests in chrome with the network inspector open, and noticed that the page still sends a request to google analytics every time tests run. This may indicate one more thing that needs stubbing. Thoughts?

request.unauthorized({});
});
stubRequest('get', url + '/v2/preprint_providers', (request) => {
request.ok({data: [{
attributes: provider.data.attributes,
type: "preprint_providers",
id: "osf"
}]});
});
stubRequest('get', url + '/v2/preprint_providers/osf', (request) => {
request.ok({data: {
attributes: provider.data.attributes,
type: "preprint_providers",
id: "osf"
}});
});

if (options.beforeEach) {
return options.beforeEach.apply(this, arguments);
}
},
afterEach() {
let afterEach = options.afterEach && options.afterEach.apply(this, arguments);
return Promise.resolve(afterEach).then(() => destroyApp(this.application));
FakeServer.stop();
let afterEach = options.afterEach && options.afterEach.apply(this, arguments);
return Promise.resolve(afterEach).then(() => destroyApp(this.application));
}
});
}
37 changes: 37 additions & 0 deletions tests/integration/components/author-link-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import Ember from 'ember';
import { moduleForComponent, test } from 'ember-qunit';
import hbs from 'htmlbars-inline-precompile';
import FactoryGuy, { manualSetup } from 'ember-data-factory-guy';

moduleForComponent('author-link', 'Integration | Component | author link', {
integration: true,
beforeEach: function() {
manualSetup(this.container);
}
});

test('renders non-links', function(assert) {
let contributorModel = FactoryGuy.make('contributor');
// Problem here is that author link expects a share search-result contributor,
// not a store instance of a contributor and its user(s).
let contributor = {users: {identifiers: []}};
contributor.users.name = contributorModel.get('users.fullName');
contributor = Ember.merge(contributor, contributorModel.serialize().data.attributes);
this.set('contributor', contributor);
this.render(hbs`{{author-link contributor=contributor}}`);
assert.ok(!this.$('a').length);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add message?

assert.equal(this.$().text().trim(), contributorModel.get('users.fullName'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add message?

});

test('renders links', function(assert) {
let contributorModel = FactoryGuy.make('contributor');
let contributor = {users: {identifiers: []}};
contributor.users.name = contributorModel.get('users.fullName');
contributor = Ember.merge(contributor, contributorModel.serialize().data.attributes);
contributor.users.identifiers.push('https://staging.osf.io/cool');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice that the component is being rendered twice.

Just to ensure tests are isolated, might I suggest splitting this into two tests? (the common setup parts could be moved into a beforeEach of course)

this.set('contributor', contributor);

this.render(hbs`{{author-link contributor=contributor}}`);
assert.ok(this.$('a').length);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add message?

assert.equal(this.$().text().trim(), contributorModel.get('users.fullName'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add message?

});
103 changes: 98 additions & 5 deletions tests/unit/controllers/content/index-test.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,105 @@
import Ember from 'ember';
import { moduleFor, test } from 'ember-qunit';
import FactoryGuy, { manualSetup } from 'ember-data-factory-guy';

moduleFor('controller:content/index', 'Unit | Controller | content/index', {
// Specify the other units that are required for this test.
needs: ['service:metrics', 'service:theme']
moduleFor('controller:content/index', 'Unit | Controller | content', {
needs: ['service:metrics', 'service:theme', 'model:file', 'model:preprint',
'model:preprint-provider', 'model:node', 'model:license',
'model:user', 'model:citation', 'model:draft-registration',
'model:contributor', 'model:comment', 'model:institution',
'model:registration', 'model:file-provider', 'model:log',
'model:node-link', 'model:wiki'],
beforeEach: function() {
manualSetup(this.container);
}
});

// Replace this with your real tests.
test('it exists', function(assert) {
let controller = this.subject();
assert.ok(controller);
let controller = this.subject();
assert.ok(controller);
});

test('fullLicenseText computed property with multiple copyright holders and year', function(assert) {
let ctrl = this.subject();
let preprint = FactoryGuy.make('preprint');
let license = preprint.get('license');
Ember.run(() => {
ctrl.set('model', preprint);
license.set('text', 'On {{year}}, for {{copyrightHolders}}');
preprint.set('licenseRecord', {
year: '2001',
copyright_holders: ['Henrique', 'Someone Else']
});
assert.equal(ctrl.get('fullLicenseText'), 'On 2001, for Henrique, Someone Else');
});
});

test('fullLicenseText computed property with no copyright holders or year', function(assert) {
let ctrl = this.subject();
let preprint = FactoryGuy.make('preprint');
let license = preprint.get('license');
Ember.run(() => {
ctrl.set('model', preprint);
license.set('text', 'On {{year}}, for {{copyrightHolders}}');
preprint.set('licenseRecord', {
year: '',
copyright_holders: []
});

assert.equal(ctrl.get('fullLicenseText'), 'On , for ');
});
});
test('fullLicenseText computed property with {{}} for copyrightHolders', function(assert) {
let ctrl = this.subject();
let preprint = FactoryGuy.make('preprint');
let license = preprint.get('license');
Ember.run(() => {
ctrl.set('model', preprint);
license.set('text', 'On {{year}}, for {{copyrightHolders}}');
preprint.set('licenseRecord', {
year: '{{year}}',
copyright_holders: ['{{copyrightHolders}}']
});

assert.equal(ctrl.get('fullLicenseText'), 'On {{year}}, for {{copyrightHolders}}');
});
});

test('useShortenedDescription computed property', function(assert) {
let ctrl = this.subject();
let node = FactoryGuy.make('node');
Ember.run(() => {
ctrl.set('node', node);

node.set('description', 'string'.repeat(100).slice(0, 351));
assert.ok(ctrl.get('useShortenedDescription'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add message?


node.set('description', 'string'.repeat(100).slice(0, 350));
assert.ok(!ctrl.get('useShortenedDescription'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add message?


node.set('description', 'string'.repeat(100).slice(0, 349));
assert.ok(!ctrl.get('useShortenedDescription'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add message?

});
});

test('description computed property', function(assert) {
let ctrl = this.subject();
let node = FactoryGuy.make('node');
Ember.run(() => {
ctrl.set('node', node);
ctrl.set('expandedAbstract', false);

//Test cut at 350 characters
let description = 'string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string string str';
let notExpanded = description.slice(0, 350);
node.set('description', description);
assert.equal(ctrl.get('description'), notExpanded.trim());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add message?


//Test cut at less than 350 characters to not cut in middle of word
description = 'string stringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstringstring';
notExpanded = 'string ';
node.set('description', description);
assert.equal(ctrl.get('description'), notExpanded.trim());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add message?

});
});
Loading