Skip to content

Refactor Maker unit tests - #51220

Merged
fisher-alice merged 4 commits into
stagingfrom
alice/refactor-maker-tests
Apr 11, 2023
Merged

Refactor Maker unit tests#51220
fisher-alice merged 4 commits into
stagingfrom
alice/refactor-maker-tests

Conversation

@fisher-alice

@fisher-alice fisher-alice commented Apr 7, 2023

Copy link
Copy Markdown
Contributor

This PR refactors Maker unit tests including the test files MakerBoardTest.js and VirtualBoardTest.js. This is a follow up to the PR that adds a virtual micro:bit board so that users can run an App Lab project for which micro:bit is enabled without a connected device.

I renamed MakerBoardTest.js to MakerBoardInterfaceTestUtil.js to better indicate that this file exports a set of tests that verify that a given board properly implements the MakerBoard interface. Thus, I removed references to specific board implementations. Currently, there are four of them - MicroBitBoard, CircuitPlayground, VirtualMBBoard, and VirtualCPBoard. These tests should be run without needing to check which specific implementation of the MakerBoard interface is being tested.

I also separated the tests in VirtualBoardTest.js into 2 different test files: VirtualMBBoardTest.js and VirtualCPBoardTest.js mirroring the 2 other existing MakerBoard test files: MicroBitBoardTest.js and CircuitPlaygroundBoardTest.js.

The last change was refactoring CircuitPlaygroundBoardTest.js and MicroBitBoardTest.js.
itMakesCircuitPlaygroundComponentsAvailable is a function that runs a set of tests that verify that the Circuit Playground board components and component constructors are available from the interpreter. The function is currently defined in and exported from CircuitPlaygroundBoardTest.js. It is called in the same file, but also called in VirtualCPBoardTest.js which seems somewhat asymmetric. I moved this function to CircuitPlaygroundComponentTestUtil.js where it is now imported in CircuitPlaygroundBoardTest.js and VirtualCPBoardTest.js.

Thus, there is greater clarity that CircuitPlaygroundBoard and VirtualCPBoard both properly implement theMakerBoard interface and both make available to the interpreter the board components and constructors.

Similar treatment was done for itMakesMicroBitComponentsAvailable.

Links

jira ticket

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@fisher-alice
fisher-alice marked this pull request as ready for review April 7, 2023 21:56
@fisher-alice
fisher-alice requested review from a team and jmkulwik April 7, 2023 21:56

@molly-moen molly-moen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, just one suggestion on commenting!

import CircuitPlaygroundBoard from '@cdo/apps/lib/kits/maker/boards/circuitPlayground/CircuitPlaygroundBoard';
import VirtualCPBoard from '@cdo/apps/lib/kits/maker/boards/VirtualCPBoard';
import MicroBitBoard from '@cdo/apps/lib/kits/maker/boards/microBit/MicroBitBoard';
import VirtualMBBoard from '@cdo/apps/lib/kits/maker/boards/VirtualMBBoard';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you add a comment to this file and the other test util file explaining that they export tests as a function? Is my understanding correct that we won't run these test files directly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure - will do! And yes, we won't run these test files directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually - there is already a comment about this on line 1 above. But I'll add to the other test files. Thanks!

@fisher-alice
fisher-alice merged commit 7c5a790 into staging Apr 11, 2023
@fisher-alice
fisher-alice deleted the alice/refactor-maker-tests branch April 11, 2023 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants