Jest and Inquirer Integration [WIP] #5

evenstensberg · 2016-12-17T14:53:01Z

Commit allows further testing and integration with inquirer and jest
for our TDD approach. Regex could be changed to have specific test
names for our purposes.

Commit allows further testing and integration with inquirer and jest for our TDD approach. Regex could be changed to have specific test names for our purposes.

Installed rxJS and our tests are running in the travis validations.

pksjce · 2016-12-17T19:21:05Z

lib/observable-questions.js

@@ -0,0 +1,12 @@
+const Rx = require('rx');
+const {inquirerInput} = require('./utils/inquirer-questions.js');


Maybe name this as inquirer-types or inquirer-question-types?

Yeah, that's probably the best, I'll change that up

pksjce · 2016-12-17T19:22:04Z

package.json

@@ -14,17 +14,32 @@
  "main": "./bin/cli.js",
  "scripts": {
    "lint": "eslint ./lib",
-    "install-commit-validator": "fit-commit-js install"
+    "install-commit-validator": "fit-commit-js install",
+    "test": "jest"


Would be good to add these info to readme.

Could do later, when we put the entire readme up?

Also good to add a test-watch script?

I think we should also lint the tests, when we don't do it, the coding style of the tests will not be consistent with the style of the code under /lib

Doing what @pksjce said we need listed probably on wednesday. Got midterms. @DanielaValero Yes! Will do! Just have to get something testable up first, collecting information so I've got enough knowledge to do this 👍

I'd also say that a watch for the test would be quite helpful when we want to code with TDD

pksjce · 2016-12-17T19:29:19Z

lib/inquirer.js

+
+inquirer.prompt(questions).ui.process.subscribe(
+	(ans) => {
+		console.log(ans);


How to test this though?

For yourself: Open in GH desktop: npm link could need a sudo flag. Type webpack-cli --init. Through jest: Simulate an inq instance if possible.

I could do it at a later stage though, this is just to set up the inquirer prompt, no need for testing in Jest yet.

Changes name to better reflect its usage.

evenstensberg · 2016-12-18T16:36:27Z

FWIW - We can mock manually or use mocking patterns to do this automatically.

Integrated new test suites and trying to get it work nicely with observable sequences.

evenstensberg · 2016-12-18T19:59:19Z

Don't merge yet, [WIP]

pksjce · 2016-12-19T18:07:23Z

lib/observable-questions.js

+	obs.next(inquirerInput('outputLogic',
+	'What is the name of the output directory in your application?'
+	));
+	obs.onCompleted();


Instead you could,

const questions = [ inquirerInput('entryLogic','What is the name of the entry point in your application?'), inquirerInput('outputLogic','What is the name of the output directory in your application?')]; module.exports = Rx.Observable.from(questions);

pksjce · 2016-12-19T18:08:29Z

tests/setup-test.js

+		 });
+		 inquirer.prompt(questions).ui.process.subscribe(function (response) {
+			 expect(response.entryLogic).toBe('index.js')
+			 expect(response.outputLogic).toBe('output.js')


I don't think you need to test if inquirer works. You only need to test what we do with the answers like they do in eslint.

DanielaValero · 2016-12-19T19:56:19Z

Hi @ev1stensberg

In spite of the work not yet being completed, therefore theoretically ready for a code review, I have done a quick check on the current state of your branch, and would like to share some ideas, perhaps you've got them already on your sight, but wanted to share them in case you don't.

In order to fix the travis error, we need to add 'use strict' on top of the test files, this is because in versions of node < 6, the block-scoped declarations of es2015 where supported only on strict mode
When we enable lintin on the test files, there will be some easy fixes that you will be able to apply with the help of the editorconfig and the beautifier, however, eslint is throwing no-undef on jasmine functions, such as describe, in order to fix them we would need to add to the list of environments in the eslint settings, jasmine: true
I'd suggest that for the test files, we use the same name of the file under ./lib, for which we are doing the testing. We could follow the same style as in webpack for the name, in there, the file has a sufix: .test.js, when we do this, for example a test for: inquirer.js, would be: inquirer.test.js. This way, would be more straight forward to find the test of the file we are at the moment looking at.

Does it make sense?

Manually mock inquirer, allows clean test stdin and a faster testing procedure.

* Jest and Inquirer Integration [WIP] (#5) * feat: testing-and-inquirer Commit allows further testing and integration with inquirer and jest for our TDD approach. Regex could be changed to have specific test names for our purposes. * feat: installed rx and test on travis Installed rxJS and our tests are running in the travis validations. * hotfix: Change name Changes name to better reflect its usage. * feat: new test suites Integrated new test suites and trying to get it work nicely with observable sequences. * fix - Adds changes to observable questions * fix - Try to pass the build * hotfix: New test pattern Manually mock inquirer, allows clean test stdin and a faster testing procedure. * hotfix: Travis stuff * Hotfix: travis * hotfix: yarn * hotfix: travis * hotfix: new travis * hotfix * hotfix2 * hotfix: Use ES6 + prepare to squash * Fix the build

Hotfix/testing (#8) * Jest and Inquirer Integration [WIP] (#5) * feat: testing-and-inquirer Commit allows further testing and integration with inquirer and jest for our TDD approach. Regex could be changed to have specific test names for our purposes. * feat: installed rx and test on travis Installed rxJS and our tests are running in the travis validations. * hotfix: Change name Changes name to better reflect its usage. * feat: new test suites Integrated new test suites and trying to get it work nicely with observable sequences. * fix - Adds changes to observable questions * fix - Try to pass the build * hotfix: New test pattern Manually mock inquirer, allows clean test stdin and a faster testing procedure. * hotfix: Travis stuff * Hotfix: travis * hotfix: yarn * hotfix: travis * hotfix: new travis * hotfix * hotfix2 * hotfix: Use ES6 + prepare to squash * Fix the build Project Restructure

* feat: testing-and-inquirer Commit allows further testing and integration with inquirer and jest for our TDD approach. Regex could be changed to have specific test names for our purposes. * feat: installed rx and test on travis Installed rxJS and our tests are running in the travis validations. * hotfix: Change name Changes name to better reflect its usage. * feat: new test suites Integrated new test suites and trying to get it work nicely with observable sequences. * fix - Adds changes to observable questions * fix - Try to pass the build * hotfix: New test pattern Manually mock inquirer, allows clean test stdin and a faster testing procedure. * hotfix: Travis stuff * Hotfix: travis * hotfix: yarn * hotfix: travis * hotfix: new travis * hotfix * hotfix2 * hotfix: Use ES6 + prepare to squash

evenstensberg added 2 commits December 17, 2016 15:52

feat: testing-and-inquirer

59ba393

Commit allows further testing and integration with inquirer and jest for our TDD approach. Regex could be changed to have specific test names for our purposes.

feat: installed rx and test on travis

5bb4001

Installed rxJS and our tests are running in the travis validations.

evenstensberg changed the title ~~feat: testing-and-inquirer~~ Jest and Inquirer Integration Dec 17, 2016

pksjce reviewed Dec 17, 2016

View reviewed changes

hotfix: Change name

9a82449

Changes name to better reflect its usage.

DanielaValero mentioned this pull request Dec 17, 2016

Webpack CLI - Define features and tasks of the package #4

Closed

3 tasks

feat: new test suites

5fcc5e0

Integrated new test suites and trying to get it work nicely with observable sequences.

evenstensberg changed the title ~~Jest and Inquirer Integration~~ Jest and Inquirer Integration [WIP] Dec 18, 2016

pksjce suggested changes Dec 19, 2016

View reviewed changes

Pavithra K and others added 11 commits December 20, 2016 19:31

fix - Adds changes to observable questions

dcc90dc

fix - Try to pass the build

ff98139

hotfix: New test pattern

da61600

Manually mock inquirer, allows clean test stdin and a faster testing procedure.

hotfix: Travis stuff

4ad2ab5

Hotfix: travis

999ec60

hotfix: yarn

8ec44a0

hotfix: travis

d2c417a

hotfix: new travis

0d3e872

hotfix

a67d97f

hotfix2

0b3a261

hotfix: Use ES6 + prepare to squash

424b320

evenstensberg changed the base branch from master to hotfix/testing December 20, 2016 17:34

evenstensberg merged commit 9e16673 into hotfix/testing Dec 20, 2016

evenstensberg deleted the feature/test-and-inquirer branch December 20, 2016 17:36

evenstensberg mentioned this pull request Dec 20, 2016

Hotfix/testing #8

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jest and Inquirer Integration [WIP] #5

Jest and Inquirer Integration [WIP] #5

evenstensberg commented Dec 17, 2016

pksjce Dec 17, 2016

evenstensberg Dec 17, 2016

pksjce Dec 17, 2016

evenstensberg Dec 17, 2016

pksjce Dec 19, 2016

DanielaValero Dec 19, 2016

evenstensberg Dec 19, 2016

DanielaValero Dec 19, 2016

pksjce Dec 17, 2016

evenstensberg Dec 17, 2016

evenstensberg Dec 17, 2016 •

edited

Loading

evenstensberg commented Dec 18, 2016

evenstensberg commented Dec 18, 2016

pksjce Dec 19, 2016

pksjce Dec 19, 2016 •

edited

Loading

DanielaValero commented Dec 19, 2016 •

edited

Loading

		@@ -0,0 +1,12 @@
		const Rx = require('rx');
		const {inquirerInput} = require('./utils/inquirer-questions.js');

Jest and Inquirer Integration [WIP] #5

Jest and Inquirer Integration [WIP] #5

Conversation

evenstensberg commented Dec 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evenstensberg Dec 17, 2016 • edited Loading

Choose a reason for hiding this comment

evenstensberg commented Dec 18, 2016

evenstensberg commented Dec 18, 2016

Choose a reason for hiding this comment

pksjce Dec 19, 2016 • edited Loading

Choose a reason for hiding this comment

DanielaValero commented Dec 19, 2016 • edited Loading

evenstensberg Dec 17, 2016 •

edited

Loading

pksjce Dec 19, 2016 •

edited

Loading

DanielaValero commented Dec 19, 2016 •

edited

Loading