phetsims / aqua

Automatic QUality Assurance
MIT License
2 stars 4 forks source link

Add node unit tests to CT #223

Closed zepumph closed 5 days ago

zepumph commented 2 weeks ago

From https://github.com/phetsims/perennial/issues/383 and https://github.com/phetsims/perennial/issues/384.

It is important to have a unit test harness for the node side of things.

zepumph commented 2 weeks ago

I believe the best way to do this is to support a new locale test type called npm run, and then we have appropriate power to handle tests that may come up in the future, without giving arbitrary control to the writer of perennial to add arbitrary node code.

zepumph commented 2 weeks ago

I believe this is going to work well. Here is an example of the tests I would add (and did locally):


tests.push( {
  test: [ 'chipper', 'qunit', 'node' ],
  type: 'npm-run',
  command: 'test',
  repo: 'chipper',
  priority: 0
} );
tests.push( {
  test: [ 'perennial', 'qunit', 'node' ],
  type: 'npm-run',
  command: 'test',
  repo: 'perennial',
  priority: 0
} );

@jonathanolson, can you give this a spot check and let me know what you think. Are we worried about opening things up to all npm run commands? I personally am really excited about being able to add more testing for node code. Let me know

zepumph commented 1 week ago

@jonathanolson and I discussed and we like the idea of locking the npm-run type test down, and restricting it to just a subset of repos.

The repos we will start with allowing this for:

Adding this directly into the server file seems great.

zepumph commented 5 days ago

Alright. I made the update to restrict repos and restarted CT on sparky. I also added tests. I'll check back in the morning.

zepumph commented 5 days ago

This is working great after the above bug fix!