Skip to content

test: Reenable a variety of skipped tests - #10320

Merged
gonfunko merged 2 commits into
mainfrom
skip-to-my-lou
Aug 28, 2026
Merged

test: Reenable a variety of skipped tests#10320
gonfunko merged 2 commits into
mainfrom
skip-to-my-lou

Conversation

@gonfunko

Copy link
Copy Markdown
Contributor

The basics

The details

Resolves

Proposed Changes

This PR removes .skip() from various tests. These are some combination of (a) tests that were passing fine but were just disabled (b) placeholder/dummy tests that were removed, (c) tests that needed some minor tweaks and (d) a suite of workspace comment tests written against a very old API that were nonetheless reasonable, and which I updated to exercise the otherwise-untested headless workspace comments.

Test Coverage

Is better now.

@gonfunko
gonfunko requested a review from a team as a code owner August 25, 2026 21:34
@gonfunko
gonfunko requested a review from maribethb August 25, 2026 21:34
});

test.skip(
'renaming a variable such that you get a parameter ' +

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.

fair enough to remove this but we probably should have a test for this behavior assuming we ever decided what the behavior should be lol, might be worth siccing the llm on it

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.

Yeah, I think the deciding what should happen is the issue. Presently it just does nothing (the variable name goes through, and becomes the same variable as the param) which I think is fine?

@@ -0,0 +1,198 @@
/**
* @license
* Copyright 2024 Google LLC

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.

new file should get new header

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.

It's not a new file, just renamed – previously there was "old_workspace_comment_test" and "workspace_comment_test"; old was renamed to workspace_comment_test since it exercises the headless regular workspace comments, and workspace_comment_test was renamed rendered_workspace_comment_test, since that's what it was exercising. That's also the cause of the copyright year change the diff shows in the other file.

/**
* @license
* Copyright 2024 Google LLC
* Copyright 2020 Google LLC

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.

this is a confusing change?

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.

Per the other comment it's an artifact of renaming, not a real change

@gonfunko
gonfunko merged commit db35dab into main Aug 28, 2026
7 checks passed
@gonfunko
gonfunko deleted the skip-to-my-lou branch August 28, 2026 18:11
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