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

QUnit testing framework #268

Closed
wants to merge 14 commits into from
Closed

QUnit testing framework #268

wants to merge 14 commits into from

Conversation

JPJPJPOPOP
Copy link
Contributor

@JPJPJPOPOP JPJPJPOPOP commented Jan 15, 2018

cc @sushain97. Tried my best at this task, and got the baseline framework down.

This provides the testing framework for QUnit. You can access in-browser testing at qunit-test.html and run tests via CLI with grunt qunit.

All testing files added to tests/ will run correctly in the browser, which you can access through qunit-test.html. However, the tests may differ from in-browser using the CLI with grunt qunit. Still trying to debug this issue, but ALL tests run in-browser work and are accurate.

Fixes #116.

JPJPJPOPOP added 3 commits Jan 15, 2018
This is because phantomJS, where we run our
continuous integration tests for QUnit, isn't on
ES6. This is the simple fix without importing other
packages.
tests/a.js Outdated
@@ -0,0 +1,5 @@
QUnit.module( "test" );
QUnit.test( "sample test 1", function (assert) {
assert.equal(4, 2+2);

Choose a reason for hiding this comment

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

No magic number: 4 no-magic-numbers
Infix operators must be spaced space-infix-ops

tests/a.js Outdated
@@ -0,0 +1,5 @@
QUnit.module( "test" );
QUnit.test( "sample test 1", function (assert) {

Choose a reason for hiding this comment

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

'QUnit' is not defined no-undef
There should be no spaces inside this paren space-in-parens
Strings must use singlequote quotes

tests/a.js Outdated
@@ -0,0 +1,5 @@
QUnit.module( "test" );

Choose a reason for hiding this comment

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

'QUnit' is not defined no-undef
There should be no spaces inside this paren space-in-parens
Strings must use singlequote quotes

Gruntfile.js Outdated
all: "build/qunit-test.html"
}
} );
grunt.loadNpmTasks( "grunt-contrib-qunit" );

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens
Strings must use singlequote quotes

Gruntfile.js Outdated
qunit: {
all: "build/qunit-test.html"
}
} );

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens

Gruntfile.js Outdated
module.exports = function( grunt ) {
grunt.initConfig( {
qunit: {
all: "build/qunit-test.html"

Choose a reason for hiding this comment

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

Identifier name 'all' is too short (< 4) id-length
Strings must use singlequote quotes

Gruntfile.js Outdated
@@ -0,0 +1,8 @@
module.exports = function( grunt ) {
grunt.initConfig( {

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens

Gruntfile.js Outdated
@@ -0,0 +1,8 @@
module.exports = function( grunt ) {

Choose a reason for hiding this comment

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

'module' is not defined no-undef
There should be no spaces inside this paren space-in-parens
Missing space before function parentheses space-before-function-paren

@shardulc
Copy link
Member

@shardulc shardulc commented Jan 15, 2018

Hound probably needs different config rules for the Gruntfile and the tests?

tests/a.js Outdated
@@ -0,0 +1,5 @@
QUnit.module('test');
QUnit.test('sample test 1', function (assert) {
assert.equal(4, 2+2);

Choose a reason for hiding this comment

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

No magic number: 4 no-magic-numbers
Infix operators must be spaced space-infix-ops

tests/a.js Outdated
@@ -0,0 +1,5 @@
QUnit.module('test');
QUnit.test('sample test 1', function (assert) {

Choose a reason for hiding this comment

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

'QUnit' is not defined no-undef

tests/a.js Outdated
@@ -0,0 +1,5 @@
QUnit.module('test');

Choose a reason for hiding this comment

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

'QUnit' is not defined no-undef

tests/a.js Outdated
@@ -0,0 +1,5 @@
QUnit.module('test');
QUnit.test('sample test 1', function (assert) {
assert.equal(4, 2+2);

Choose a reason for hiding this comment

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

No magic number: 4 no-magic-numbers
Infix operators must be spaced space-infix-ops

tests/a.js Outdated
@@ -0,0 +1,5 @@
QUnit.module('test');
QUnit.test('sample test 1', function (assert) {

Choose a reason for hiding this comment

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

'QUnit' is not defined no-undef

tests/a.js Outdated
@@ -0,0 +1,5 @@
QUnit.module('test');

Choose a reason for hiding this comment

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

'QUnit' is not defined no-undef

@sushain97
Copy link
Member

@sushain97 sushain97 commented Jan 15, 2018

Just make Hound ignore the tests folder? I don't think there's a way to configure different rulesets. Some of the errors should be fixed though.

Copy link
Member

@sushain97 sushain97 left a comment

Looks like a good start! Excited to see a real test ;)

@@ -8,7 +8,9 @@ jobs:

# Install development dependencies
- run: sudo apt-get install python3 python3-pip
- run: sudo npm install -g jsonlint eslint flow flow-bin flow-coverage-report htmlhint sass-lint
- run: sudo npm install -g jsonlint eslint flow flow-bin flow-coverage-report htmlhint sass-lint grunt-cli
Copy link
Member

@sushain97 sushain97 Jan 15, 2018

Choose a reason for hiding this comment

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

We should just have a package.json at this point.

Copy link
Contributor Author

@JPJPJPOPOP JPJPJPOPOP Jan 17, 2018

Choose a reason for hiding this comment

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

@sushain97 I tried for several hours to try to get this to work, but I was unable to do this. I don't exactly know, but it might be because of the -g, and apparently installing “global” npm dependencies via package.json isn't supported? I created the package.json and was running npm install, but I kept on getting No such file or directory.

Copy link

@Krinkle Krinkle Nov 27, 2020

Choose a reason for hiding this comment

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

Looks like this has since been done in latest master!

@@ -23,6 +25,9 @@ jobs:
# Build project
- run: cp config.conf.example config.conf
- run: make -j8 || true

# Run unit tests
- run: grunt qunit
Copy link
Member

@sushain97 sushain97 Jan 15, 2018

Choose a reason for hiding this comment

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

Move this up to with the run tests block.

Copy link
Contributor Author

@JPJPJPOPOP JPJPJPOPOP Jan 15, 2018

Choose a reason for hiding this comment

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

This has to be done after make -j8 is called.

Copy link
Member

@sushain97 sushain97 Jan 15, 2018

Choose a reason for hiding this comment

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

Fair, then let's move the make stuff up.

Makefile Outdated
@@ -5,13 +5,13 @@ all: check-deps prod

debug: debugjs debugcss build/index.debug.html build/not-found.html fonts build/js/compat.js build/js/jquery.min.js build/js/bootstrap.min.js build/sitemap.xml build/strings/locales.json build/index.$(DEFAULT_LOCALE).html build/strings/$(DEFAULT_LOCALE).json images

prod: js css html fonts build/sitemap.xml build/manifest.json build/strings/locales.json localhtml images
prod: js css html fonts build/sitemap.xml build/manifest.json build/strings/locales.json localhtml images tests
Copy link
Member

@sushain97 sushain97 Jan 15, 2018

Choose a reason for hiding this comment

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

No, this doesn't sound good. We should have a test target, a la make test.

@@ -100,7 +100,7 @@ $(document).ready(function () {

var pathname /*: string */ = window.location.pathname;

if(window.history.replaceState && !pathname.endsWith('/index.debug.html')) {
if(window.history.replaceState && !(pathname.indexOf('/index.debug.html') === pathname.length - '/index.debug.html'.length)) {
Copy link
Member

@sushain97 sushain97 Jan 15, 2018

Choose a reason for hiding this comment

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

what's going on here?

Copy link
Contributor Author

@JPJPJPOPOP JPJPJPOPOP Jan 15, 2018

Choose a reason for hiding this comment

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

@sushain97 This is because phantomJS, where we run our continuous integration tests for QUnit, isn't on ES6. This is the simple fix without importing other packages.

Copy link
Member

@sushain97 sushain97 Jan 15, 2018

Choose a reason for hiding this comment

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

Doesn't compat.js have shims? It could be included in the test head to fix it.

@@ -44,7 +44,7 @@ Store.prototype.set = function /*:: <T> */ (key /*: string */, value /*: T */) /
Store.prototype.clear = function () /*: void */ {
if(this.able()) {
for(var key in window.localStorage) {
if(key.startsWith((this.prefix /*: string */))) {
if(key.indexOf((this.prefix /*: string */)) === 0) {
Copy link
Member

@sushain97 sushain97 Jan 15, 2018

Choose a reason for hiding this comment

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

startsWith is much nicer?? why change this (and the other instances)

index.html.in Outdated
@@ -58,6 +59,7 @@
<link rel="manifest" href="./manifest.json">
</head>
<body>
<!-- qunit-testing -->
Copy link
Member

@sushain97 sushain97 Jan 15, 2018

Choose a reason for hiding this comment

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

quint-tests?

@@ -31,6 +31,7 @@

<!--[if lt IE 9]> <script type="text/javascript" src="./js/compat.js"></script> <![endif]-->
@include_head@
<!-- qunit head -->
Copy link
Member

@sushain97 sushain97 Jan 15, 2018

Choose a reason for hiding this comment

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

qunit-head? I'm hoping this only happens with make test, we don't need everyone running tests ;)

Copy link
Contributor Author

@JPJPJPOPOP JPJPJPOPOP Jan 17, 2018

Choose a reason for hiding this comment

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

@sushain97 The <!-- qunit head --> and <!-- qunit-tests -->only get injected when running on qunit-test.html.

@apertium apertium deleted a comment from houndci-bot Jan 18, 2018
@apertium apertium deleted a comment from houndci-bot Jan 18, 2018
@apertium apertium deleted a comment from houndci-bot Jan 18, 2018
@apertium apertium deleted a comment from houndci-bot Jan 18, 2018
tests/a.js Outdated
@@ -0,0 +1,23 @@
/* eslint-disable */
Copy link
Member

@sushain97 sushain97 Jan 18, 2018

Choose a reason for hiding this comment

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

Why? There are a number of stylistic concerns here.

qunit-test.html Outdated


<!-- add test files here -->
<script type="text/javascript" src="./tests/a.js"></script>
Copy link
Member

@sushain97 sushain97 Jan 18, 2018

Choose a reason for hiding this comment

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

a?

# Build project
- run: cp config.conf.example config.conf
- run: make -j8 || true
- run: grunt qunit
Copy link
Member

@sushain97 sushain97 Jan 18, 2018

Choose a reason for hiding this comment

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

make test never gets run?

Copy link
Contributor Author

@JPJPJPOPOP JPJPJPOPOP Jan 18, 2018

Choose a reason for hiding this comment

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

@sushain97 did you want make to not create test files and make test to create test files?

Copy link
Member

@sushain97 sushain97 Jan 18, 2018

Choose a reason for hiding this comment

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

Seems sensible?

Copy link
Contributor Author

@JPJPJPOPOP JPJPJPOPOP Jan 18, 2018

Choose a reason for hiding this comment

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

In other words, currently make test works to create test files. However, make will always build all files for you including test files.

Copy link
Contributor Author

@JPJPJPOPOP JPJPJPOPOP Jan 18, 2018

Choose a reason for hiding this comment

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

@sushain97 Ahh oops, I realized what I did wrong :D

tests/test.js Outdated
assert.equal( config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('generation'), modeEnabled('generation'), 'Generation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('analyzation'), modeEnabled('analyzation'), 'Analyzation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('sandbox'), modeEnabled('sandbox'), 'Sandbox mode is consistent' );

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 3 indent
There should be no spaces inside this paren space-in-parens
'config' is not defined no-undef
'modeEnabled' is not defined no-undef

tests/test.js Outdated
assert.expect( 4 );
assert.equal( config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('generation'), modeEnabled('generation'), 'Generation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('analyzation'), modeEnabled('analyzation'), 'Analyzation mode is consistent' );

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 3 indent
There should be no spaces inside this paren space-in-parens
'config' is not defined no-undef
'modeEnabled' is not defined no-undef

tests/test.js Outdated
QUnit.test( 'Check modeEnabled', function ( assert ) {
assert.expect( 4 );
assert.equal( config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('generation'), modeEnabled('generation'), 'Generation mode is consistent' );

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 3 indent
There should be no spaces inside this paren space-in-parens
'config' is not defined no-undef
'modeEnabled' is not defined no-undef

tests/test.js Outdated
QUnit.module( 'Verify modeEnabled is consistent with config file' );
QUnit.test( 'Check modeEnabled', function ( assert ) {
assert.expect( 4 );
assert.equal( config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent' );

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 3 indent
There should be no spaces inside this paren space-in-parens
'config' is not defined no-undef
'modeEnabled' is not defined no-undef

tests/test.js Outdated

QUnit.module( 'Verify modeEnabled is consistent with config file' );
QUnit.test( 'Check modeEnabled', function ( assert ) {
assert.expect( 4 );

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 3 indent
There should be no spaces inside this paren space-in-parens
No magic number: 4 no-magic-numbers

tests/test.js Outdated
$( 'a[data-mode="sandbox"]' ).click();
assert.equal( $('#sandboxContainer').css('display'), 'block', 'Switching to sandbox mode' );
$( 'a[data-mode="translation"]' ).click();
assert.equal( $('#translationContainer').css('display'), 'block', 'Switching to translation mode' );

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens

tests/test.js Outdated
assert.expect( 4 );
$( 'a[data-mode="sandbox"]' ).click();
assert.equal( $('#sandboxContainer').css('display'), 'block', 'Switching to sandbox mode' );
$( 'a[data-mode="translation"]' ).click();

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens

tests/test.js Outdated
QUnit.test( 'Mode test', function ( assert ) {
assert.expect( 4 );
$( 'a[data-mode="sandbox"]' ).click();
assert.equal( $('#sandboxContainer').css('display'), 'block', 'Switching to sandbox mode' );

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens

tests/test.js Outdated
QUnit.module( 'Change mode' );
QUnit.test( 'Mode test', function ( assert ) {
assert.expect( 4 );
$( 'a[data-mode="sandbox"]' ).click();

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens

tests/test.js Outdated
@@ -0,0 +1,21 @@
QUnit.module( 'Change mode' );
QUnit.test( 'Mode test', function ( assert ) {
assert.expect( 4 );

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens
No magic number: 4 no-magic-numbers

tests/test.js Outdated
assert.equal( config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('generation'), modeEnabled('generation'), 'Generation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('analyzation'), modeEnabled('analyzation'), 'Analyzation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('sandbox'), modeEnabled('sandbox'), 'Sandbox mode is consistent' );

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 3 indent
There should be no spaces inside this paren space-in-parens
'config' is not defined no-undef
'modeEnabled' is not defined no-undef

tests/test.js Outdated
assert.expect( 4 );
assert.equal( config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('generation'), modeEnabled('generation'), 'Generation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('analyzation'), modeEnabled('analyzation'), 'Analyzation mode is consistent' );

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 3 indent
There should be no spaces inside this paren space-in-parens
'config' is not defined no-undef
'modeEnabled' is not defined no-undef

tests/test.js Outdated
QUnit.test( 'Check modeEnabled', function ( assert ) {
assert.expect( 4 );
assert.equal( config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent' );
assert.equal( config.ENABLED_MODES.includes('generation'), modeEnabled('generation'), 'Generation mode is consistent' );

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 3 indent
There should be no spaces inside this paren space-in-parens
'config' is not defined no-undef
'modeEnabled' is not defined no-undef

tests/test.js Outdated
QUnit.module( 'Verify modeEnabled is consistent with config file' );
QUnit.test( 'Check modeEnabled', function ( assert ) {
assert.expect( 4 );
assert.equal( config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent' );

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 3 indent
There should be no spaces inside this paren space-in-parens
'config' is not defined no-undef
'modeEnabled' is not defined no-undef

tests/test.js Outdated

QUnit.module( 'Verify modeEnabled is consistent with config file' );
QUnit.test( 'Check modeEnabled', function ( assert ) {
assert.expect( 4 );

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 3 indent
There should be no spaces inside this paren space-in-parens
No magic number: 4 no-magic-numbers

tests/test.js Outdated
$( 'a[data-mode="sandbox"]' ).click();
assert.equal( $('#sandboxContainer').css('display'), 'block', 'Switching to sandbox mode' );
$( 'a[data-mode="translation"]' ).click();
assert.equal( $('#translationContainer').css('display'), 'block', 'Switching to translation mode' );

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens

tests/test.js Outdated
assert.expect( 4 );
$( 'a[data-mode="sandbox"]' ).click();
assert.equal( $('#sandboxContainer').css('display'), 'block', 'Switching to sandbox mode' );
$( 'a[data-mode="translation"]' ).click();

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens

tests/test.js Outdated
QUnit.test( 'Mode test', function ( assert ) {
assert.expect( 4 );
$( 'a[data-mode="sandbox"]' ).click();
assert.equal( $('#sandboxContainer').css('display'), 'block', 'Switching to sandbox mode' );

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens

tests/test.js Outdated
QUnit.module( 'Change mode' );
QUnit.test( 'Mode test', function ( assert ) {
assert.expect( 4 );
$( 'a[data-mode="sandbox"]' ).click();

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens

tests/test.js Outdated
@@ -0,0 +1,21 @@
QUnit.module( 'Change mode' );
QUnit.test( 'Mode test', function ( assert ) {
assert.expect( 4 );

Choose a reason for hiding this comment

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

There should be no spaces inside this paren space-in-parens
No magic number: 4 no-magic-numbers

assert.equal(config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent');
assert.equal(config.ENABLED_MODES.includes('generation'), modeEnabled('generation'), 'Generation mode is consistent');
assert.equal(config.ENABLED_MODES.includes('analyzation'), modeEnabled('analyzation'), 'Analyzation mode is consistent');
assert.equal(config.ENABLED_MODES.includes('sandbox'), modeEnabled('sandbox'), 'Sandbox mode is consistent');

Choose a reason for hiding this comment

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

'config' is not defined no-undef
'modeEnabled' is not defined no-undef

assert.expect(4);
assert.equal(config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent');
assert.equal(config.ENABLED_MODES.includes('generation'), modeEnabled('generation'), 'Generation mode is consistent');
assert.equal(config.ENABLED_MODES.includes('analyzation'), modeEnabled('analyzation'), 'Analyzation mode is consistent');

Choose a reason for hiding this comment

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

'config' is not defined no-undef
'modeEnabled' is not defined no-undef

QUnit.test('Check modeEnabled', function (assert) {
assert.expect(4);
assert.equal(config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent');
assert.equal(config.ENABLED_MODES.includes('generation'), modeEnabled('generation'), 'Generation mode is consistent');

Choose a reason for hiding this comment

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

'config' is not defined no-undef
'modeEnabled' is not defined no-undef

QUnit.module('Verify modeEnabled is consistent with config file');
QUnit.test('Check modeEnabled', function (assert) {
assert.expect(4);
assert.equal(config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent');

Choose a reason for hiding this comment

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

'config' is not defined no-undef
'modeEnabled' is not defined no-undef

tests/test.js Outdated

QUnit.module('Verify modeEnabled is consistent with config file');
QUnit.test('Check modeEnabled', function (assert) {
assert.expect(4);

Choose a reason for hiding this comment

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

No magic number: 4 no-magic-numbers

tests/test.js Outdated
@@ -0,0 +1,21 @@
QUnit.module('Change mode');
QUnit.test('Mode test', function (assert) {
assert.expect(4);

Choose a reason for hiding this comment

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

No magic number: 4 no-magic-numbers

@apertium apertium deleted a comment from houndci-bot Jan 18, 2018
@apertium apertium deleted a comment from houndci-bot Jan 18, 2018
@apertium apertium deleted a comment from houndci-bot Jan 18, 2018
@apertium apertium deleted a comment from houndci-bot Jan 18, 2018
assert.equal(config.ENABLED_MODES.includes('translation'), modeEnabled('translation'), 'Translation mode is consistent');
assert.equal(config.ENABLED_MODES.includes('generation'), modeEnabled('generation'), 'Generation mode is consistent');
assert.equal(config.ENABLED_MODES.includes('analyzation'), modeEnabled('analyzation'), 'Analyzation mode is consistent');
assert.equal(config.ENABLED_MODES.includes('sandbox'), modeEnabled('sandbox'), 'Sandbox mode is consistent');
Copy link
Member

@sushain97 sushain97 Jan 18, 2018

Choose a reason for hiding this comment

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

How does this test pass? sandbox isn't enabled in the default configuration...

Copy link
Contributor Author

@JPJPJPOPOP JPJPJPOPOP Jan 18, 2018

Choose a reason for hiding this comment

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

@sushain97 Not exactly sure what you mean. It should pass, because both would be false then.

# Build project
- run: cp config.conf.example config.conf
- run: make -j8 || true
- run: make test
Copy link
Member

@sushain97 sushain97 Jan 21, 2018

Choose a reason for hiding this comment

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

Seems like make test is a superset of make, i.e. these could be combined?

Gruntfile.js Outdated
grunt.initConfig({
qunit: {
all: 'build/qunit-test.html'
}
Copy link
Member

@sushain97 sushain97 Jan 21, 2018

Choose a reason for hiding this comment

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

missing trailing commas

Gruntfile.js Outdated
@@ -0,0 +1,10 @@
/* eslint-disable */

module.exports = function(grunt) {
Copy link
Member

@sushain97 sushain97 Jan 21, 2018

Choose a reason for hiding this comment

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

space between function and (?

@sushain97
Copy link
Member

@sushain97 sushain97 commented Jan 21, 2018

Hound should be disabled for the Gruntfile?

@JPJPJPOPOP
Copy link
Contributor Author

@JPJPJPOPOP JPJPJPOPOP commented Jan 21, 2018

@sushain97 I took out the disable temporarily to see if there were any other formatting issues that I may have missed :D, forgot to add back before pushing

@apertium apertium deleted a comment from houndci-bot Mar 25, 2018
@apertium apertium deleted a comment from houndci-bot Mar 25, 2018
@apertium apertium deleted a comment from houndci-bot Mar 25, 2018
@TinoDidriksen TinoDidriksen force-pushed the master branch from 0f5d286 to