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

chore: clean up mutator tests #7434

Merged
merged 8 commits into from
Aug 28, 2023

Conversation

BeksOmega
Copy link
Collaborator

@BeksOmega BeksOmega commented Aug 24, 2023

The basics

The details

Resolves

Fixes N/A

Follow on to #7423 that fixes up the mutator tests.

Proposed Changes

  • Reformats the mutator tests to conform to the style guide.
  • Changes the connect method to not need to take in the element selector.
  • Adds some helpers to remove more concrete selectors from the test.

Reason for Changes

All of these things are to aid in readability.

Changes to the connect method also involved partially fixing #7432, so we may want to delay those changes for now since they're not complete.

Test Coverage

This is tests!

Documentation

N/A

Additional Information

N/A
](https://github.com/google/blockly/pull/7393/files)

@BeksOmega BeksOmega requested a review from a team as a code owner August 24, 2023 18:40
@BeksOmega BeksOmega requested a review from NeilFraser August 24, 2023 18:40
@github-actions github-actions bot added PR: chore General chores (dependencies, typos, etc) and removed PR: chore General chores (dependencies, typos, etc) labels Aug 24, 2023
suiteSetup(async function () {
this.browser = await testSetup(testFileLocations.PLAYGROUND);
});

test('This test mutating a block creates more inputs', async function () {
await testingMutator(this.browser, screenDirection.LTR);
test.only('Mutating a block creates more inputs', async function () {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Remove only.

Comment on lines 314 to 326
if (mutatorBlockId) {
draggedLocation = await getLocationOfBlockConnection(
browser,
draggedBlock,
draggedBlock.id,
draggedConnection,
mutatorBlockId,
);
targetLocation = await getLocationOfBlockConnection(
browser,
targetBlock,
targetBlock.id,
targetConnection,
mutatorBlockId,
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we can remove this else and just always pass mutatorBlockId (which may be undefined).

@@ -1214,6 +1214,8 @@ export abstract class Flyout extends DeleteArea implements IFlyout {

// Clone the block.
const json = blocks.save(oldBlock) as blocks.State;
// TODO: Add a saveIds parameter to `save`.
json['id'] = undefined;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm going to revert this change because I don't want to half-fix the issue.

@BeksOmega BeksOmega enabled auto-merge (squash) August 28, 2023 21:54
@BeksOmega BeksOmega merged commit 8193cff into google:develop Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: chore General chores (dependencies, typos, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants