Skip to content

dependency: update trash dep and convert trash files from JS to TS #31667

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
May 9, 2025
Merged

dependency: update trash dep and convert trash files from JS to TS #31667

merged 15 commits into from
May 9, 2025

Conversation

jennifer-shehane
Copy link
Member

@jennifer-shehane jennifer-shehane commented May 8, 2025 •
edited
Loading

Additional details

I was originally looking into options to fix this issue: #24582 But that would require our server package supporting ESM.

This just updates the trash dep and converts the files to TS as a chore update basically.

Steps to test

The main application of this trash function is when trashAssetsBeforeRuns is set to true, the screenshotsFolder, videosFolder, and downloadsFolder CONTENTS should be trashed/emptied (depending on OS). So this could be manually tested.

I tested this locally on my Mac, that the previous contents were removed on a new run in run mode.

First run

Screenshot 2025-05-09 at 12 03 11 PM

Second run

Screenshot 2025-05-09 at 12 05 26 PM

Example ran

<!DOCTYPE html>
<html>
  <body>
    <h1>Download txt file</h1>
    <p>
      Click on the link to download a text file.
      <a href="file2.txt" download>file.txt</a>
    </p>
  </body>
</html>

file.txt

This is a text file

cypress.config.js

const { defineConfig } = require("cypress");

module.exports = defineConfig({
  video: true
});

spec.cy.js

it('downloads a text file', () => {
  cy.visit('index.html')
  cy.screenshot('docs-search')
  cy.get('a[download]').click()
  cy.readFile('cypress/downloads/file.txt').should(
    'equal',
    'This is a text file\n',
  )
})

How has the user experience changed?

Users might see some bug fixes or performance improvements with trashing on Linux as some of those fixes were included in the trash 6.x and 7.x updates. https://github.com/sindresorhus/trash/releases

PR Tasks

@jennifer-shehane jennifer-shehane self-assigned this May 8, 2025
@cypress Cypress
Copy link

cypress bot commented May 8, 2025 •
edited
Loading

cypress    Run #62305

Run Properties:  status check passed Passed #62305  •  git commit cd0eeb877a: Merge branch 'update-trash' of https://github.com/cypress-io/cypress into update...
Project cypress
Branch Review update-trash
Run status status check passed Passed #62305
Run duration 18m 17s
Commit git commit cd0eeb877a: Merge branch 'update-trash' of https://github.com/cypress-io/cypress into update...
Committer Jennifer Shehane
View all properties for this run ↗︎
Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 781
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 6345
View all changes introduced in this branch ↗︎
UI Coverage  0%
  Untested elements 4  
  Tested elements 0  
Accessibility  97.09%
  Failed rules  0 critical   1 serious   0 moderate   0 minor
  Failed elements 6  

jennifer-shehane
jennifer-shehane commented May 9, 2025
View reviewed changes
packages/server/test/unit/util/trash_spec.ts
})

context('.folder', () => {
it('trashes contents of directory in non-Linux', async () => {
Copy link
Member Author

@jennifer-shehane jennifer-shehane May 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This test used to use mock-fs, but it would literally fail every time you ran it locally on a mac because the trash code was literally trying to trash a file that never existed, so this has been updated to just make temp directories during the tests outright.

jennifer-shehane
jennifer-shehane commented May 9, 2025
View reviewed changes
scripts/binary/binary-cleanup.js
@@ -51,6 +51,7 @@ const getDependencyPathsToKeep = async (buildAppDir) => {
'node_modules/html-webpack-plugin-5/index.js',
'node_modules/mocha-7.2.0/index.js',
'packages/server/node_modules/webdriver/build/index.js',
'packages/server/node_modules/@wdio/utils/build/node.js',
Copy link
Member Author

Choose a reason for hiding this comment

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

See this thread: https://cypressio.slack.com/archives/C06G5D5KFCJ/p1746724397061149

jennifer-shehane
jennifer-shehane commented May 9, 2025
View reviewed changes
packages/server/lib/util/trash.ts
@@ -0,0 +1,34 @@
import { fs } from './fs'
Copy link
Member Author

Choose a reason for hiding this comment

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

This was rewritten a bit, so review carefully

@jennifer-shehane jennifer-shehane merged commit 2d7a84d into develop May 9, 2025
125 of 127 checks passed
@jennifer-shehane jennifer-shehane deleted the update-trash branch May 9, 2025 17:14
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 20, 2025

Released in 14.4.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v14.4.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators May 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ryanthemanuel ryanthemanuel ryanthemanuel approved these changes

Assignees

@jennifer-shehane jennifer-shehane

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jennifer-shehane @ryanthemanuel