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

[Sprite Lab] Test for workspace alert #45099

Merged
merged 3 commits into from Mar 2, 2022
Merged

Conversation

mikeharv
Copy link
Contributor

@mikeharv mikeharv commented Mar 1, 2022

This is a follow-up to #45055, which adds a workspace alert for execution errors in Sprite Lab. We should also add a test to be sure this is working as intended.

Due to a naming conflict, we can rename an existing use of studioApp and reserve that label for the singleton that is imported. (See new line 23 and changes to lines ~43 to ~65).

Created Jira ticket STAR-2127 as a subtask of STAR-2099 to track.

@mikeharv mikeharv requested a review from a team March 1, 2022 19:23
@@ -40,11 +41,11 @@ describe('SpriteLab', () => {
});
afterEach(() => document.body.removeChild(container));

let studioApp;
let testStudioApp;
Copy link
Contributor

Choose a reason for hiding this comment

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

naming nit: in unit tests a "fake" version of something usually called a "mock"; this object is a mock version of StudioApp, so the convention would be mockStudioApp (which also means your rename here is how it originally should've been named!)

Copy link
Contributor

Choose a reason for hiding this comment

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

there are multiple ways to fake things in unit tests, too, as you've shown in your changes here. this is a good article about the difference between the two most common fakes, mocks and stubs: https://martinfowler.com/articles/mocksArentStubs.html

Comment on lines 154 to 156
workspaceAlertError: () => {
return 'translated string';
}
Copy link
Contributor

Choose a reason for hiding this comment

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

we can simplify this expression with ES6 arrow function expressions

apps/test/unit/p5lab/spritelab/SpritelabTest.js Outdated Show resolved Hide resolved
apps/test/unit/p5lab/spritelab/SpritelabTest.js Outdated Show resolved Hide resolved
@mikeharv mikeharv merged commit a381c36 into staging Mar 2, 2022
@mikeharv mikeharv deleted the test-for-workspace-alert branch March 2, 2022 18:48
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.

None yet

2 participants