Skip to content

Conversation

@chaoran-chen
Copy link
Member

@chaoran-chen chaoran-chen commented Jan 11, 2026

This removes unused code in the integration tests.

(This is using the branch from #5827 as base to avoid conflicts.)

🚀 Preview: Add preview label to enable

@chaoran-chen chaoran-chen changed the title feat(website): add file uploads to revisions refactor(integration-tests): remove unused methods Jan 11, 2026
@theosanderson
Copy link
Member

At first glance I'm not sure about this for most of these. These are all .page.ts files.

I think it's fine for them to have functions that aren't used atm. IMO in their ideal state these files would capture every single functionality of the page in question.

Practical reasons to keep them:

  • we might want to use them later
  • (more relevant for me) they describe how to interact with these pages using playwright to AI agents

@theosanderson
Copy link
Member

But maybe we've ended up with a lot of duplication and maybe this broad brush approach is the easiest way to fix it, idk

@chaoran-chen
Copy link
Member Author

chaoran-chen commented Jan 11, 2026

The problem is that we don't know whether they are actually correct and they can easily get out-of-date. I stumbled over this here where Claude called the submitRevision function but it's actually wrong (there is no confirm button) and it took me a while to realize that the function had been unused (what made it more difficult was that the function itself was not directly unused but the functions that used it were).

@theosanderson
Copy link
Member

The problem is that we don't know whether they are actually correct and they can easily get out-of-date.

I guess IMO this is true of features that we haven't yet added tests for too - but my solution there would be to add tests rather than remove them (and same here). [It's not a huge deal either way though, and I definitely agree with a good proportion of the removals]

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.

3 participants