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

Remove outdated vscode-ui tests (being replaced with e2e tests) #2584

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sagerb
Copy link
Collaborator

@sagerb sagerb commented Feb 10, 2025

Intent

Resolves #2585

Type of Change

    • Bug Fix
    • New Feature
    • Breaking Change
    • Documentation
    • Refactor
    • Tooling

Approach

Simply removed.

User Impact

No user impact.

Automated Tests

Directions for Reviewers

Successful CI should validate this enough.

Checklist

@sagerb sagerb changed the title WIP: remove outdated vscode-ui tests. Being replaced with e2e tests. Remove outdated vscode-ui tests (being replaced with e2e tests) Feb 10, 2025
@sagerb sagerb marked this pull request as ready for review February 10, 2025 19:57
@jonkeane
Copy link
Collaborator

I went looking (though haven't literally RTFSed yet), are the cases that were tested here already written with the new infrastructure?

@sagerb
Copy link
Collaborator Author

sagerb commented Feb 11, 2025

I went looking (though haven't literally RTFSed yet), are the cases that were tested here already written with the new infrastructure?

Not quite. I've created an issue that calls this out: #2586 (and outlines what the existing tests were doing).

Since we're not in a rush for the existing PRs blocked by this removal (so that CI will pass), I would feel better if we implemented #2586 ahead of removing these (as long as we can get CI to pass on them). However, it will mean that several of the PRs will be blocked, and that gives me pause. One benefit of prioritizing this would be that the new e2e framework would quickly be used (which would leverage the recent focus that Marcos has on that infrastructure).

I believe this PR can wait on #2586 and I'll tag it as blocked.

@sagerb sagerb added the blocked Blocked on something label Feb 11, 2025
@sagerb sagerb self-assigned this Feb 11, 2025
@sagerb sagerb removed the blocked Blocked on something label Feb 22, 2025
@sagerb
Copy link
Collaborator Author

sagerb commented Feb 22, 2025

Replacement tests are now ready to merge: #2590.

Once approved, we can merge this in now.

Copy link
Collaborator

@marcosnav marcosnav left a comment

Choose a reason for hiding this comment

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

🆗

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.

Remove legacy vscode-ui tests
3 participants