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

Add node unit tests to CT #223

Closed
zepumph opened this issue Nov 13, 2024 · 5 comments
Closed

Add node unit tests to CT #223

zepumph opened this issue Nov 13, 2024 · 5 comments

Comments

@zepumph
Copy link
Member

zepumph commented Nov 13, 2024

From phetsims/perennial#383 and phetsims/perennial#384.

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

@zepumph
Copy link
Member Author

zepumph commented Nov 13, 2024

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 added a commit that referenced this issue Nov 13, 2024
zepumph added a commit that referenced this issue Nov 13, 2024
zepumph added a commit that referenced this issue Nov 13, 2024
@zepumph
Copy link
Member Author

zepumph commented Nov 13, 2024

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
Copy link
Member Author

zepumph commented Nov 19, 2024

@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:

  • perennial
  • chipper
  • yotta
  • aqua

Adding this directly into the server file seems great.

@zepumph
Copy link
Member Author

zepumph commented Nov 21, 2024

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 added a commit that referenced this issue Nov 21, 2024
@zepumph
Copy link
Member Author

zepumph commented Nov 22, 2024

This is working great after the above bug fix!

@zepumph zepumph closed this as completed Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants