Refactor Maker unit tests - #51220
Merged
Merged
Conversation
fisher-alice
marked this pull request as ready for review
April 7, 2023 21:56
molly-moen
approved these changes
Apr 10, 2023
molly-moen
left a comment
Contributor
There was a problem hiding this comment.
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'; |
Contributor
There was a problem hiding this comment.
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?
Contributor
Author
There was a problem hiding this comment.
Sure - will do! And yes, we won't run these test files directly.
Contributor
Author
There was a problem hiding this comment.
Actually - there is already a comment about this on line 1 above. But I'll add to the other test files. Thanks!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR refactors Maker unit tests including the test files
MakerBoardTest.jsandVirtualBoardTest.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.jstoMakerBoardInterfaceTestUtil.jsto 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, andVirtualCPBoard. 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.jsinto 2 different test files:VirtualMBBoardTest.jsandVirtualCPBoardTest.jsmirroring the 2 other existing MakerBoard test files:MicroBitBoardTest.jsandCircuitPlaygroundBoardTest.js.The last change was refactoring
CircuitPlaygroundBoardTest.jsandMicroBitBoardTest.js.itMakesCircuitPlaygroundComponentsAvailableis 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 fromCircuitPlaygroundBoardTest.js. It is called in the same file, but also called inVirtualCPBoardTest.jswhich seems somewhat asymmetric. I moved this function toCircuitPlaygroundComponentTestUtil.jswhere it is now imported inCircuitPlaygroundBoardTest.jsandVirtualCPBoardTest.js.Thus, there is greater clarity that
CircuitPlaygroundBoardandVirtualCPBoardboth properly implement theMakerBoardinterface 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: