search

JBaczuk / bitcoin-script-js

Bitcoin script interpreter written in javascript for browsers and node.js
MIT License
1 stars 1 forks source link

add browser test framework. Implements #6 #13

Closed JBaczuk closed 5 years ago

JBaczuk commented 5 years ago

This will run the unit tests in the browsers specified in the karma.conf.js file (currently chrome and firefox):

$ npm i
$ npm run browser-test
JBaczuk commented 5 years ago

Oops yeah, I don't know what happened there. Should be good now (force pushed).

wardlem commented 5 years ago

@JBaczuk What are your thoughts on using this instead of explicitly setting firefox and chrome as browsers? https://www.npmjs.com/package/karma-detect-browsers

wardlem commented 5 years ago

I wasn't able to test if it worked with Chrome, since I don't have it installed on this machine. Firefox worked well though.

The karma.conf.js file needs should be linted before this is merged into master.

wardlem commented 5 years ago

A little late, but I did notice one other little thing.

This line: 

files: [
      'test/unit/*.js'
    ],

In the package.json file, I wrote the file pattern as test/unit/*.test.js with a recursive flag set. These two configurations will select two (potentially) different sets of test files.

Bee-Tee-Dub (BTW): I like to set my test files to have the suffix .test.js for two reasons:

  1. You can place helper files in the same directory as your test and they are not treated as test files.
  2. I tend to name my test files after the files they are testing (i.e. thing.js => thing.test.js) and it is easier to distinguish which file is the source file and which file is the text file when doing a quick lookup in an editor.

However, this is not a convention that we need to use for this project if you don't want to. I jumped in and started making executive decisions hoping you would veto them if you didn't like the choices I made.

JBaczuk commented 5 years ago

@JBaczuk What are your thoughts on using this instead of explicitly setting firefox and chrome as browsers? https://www.npmjs.com/package/karma-detect-browsers

It's probably fine as long as we set up a TravisCI build that tests all of the browsers we want to be compatible with. Opened #18.

However, this is not a convention that we need to use for this project if you don't want to. I jumped in and started making executive decisions hoping you would veto them if you didn't like the choices I made.

I like it, and I really appreciate your insight, and please make all the decisions you think would be good, I will let you know if I disagree.