abramenal / cypress-file-upload

File upload testing made easy
https://npm.im/cypress-file-upload
MIT License
496 stars 89 forks source link

Update Blob use to be compatible with Cypress 5.0 #215

Closed emilong closed 3 years ago

emilong commented 3 years ago

Wraps return value of Cypress.Blob.base64StringToBlob() in a Cypress.Promise.resolve, which will allow us to treat it as a Promise no matter what. If it's already a Promise, as it was in Cypress <=4, this will be also be compatible.

Fixes #214

mduft commented 3 years ago

Any way I can test this? I tried:

EtienneBruines commented 3 years ago

Cypress adds a .then function to the result of e.g. base64StringToBlob

Soure: https://github.com/cypress-io/cypress/blob/de175d910d02c8d082b05883885c8a7aa1fb27b7/packages/driver/src/util/breaking_change_warning.ts#L18

        val.then = function () {
          $errUtil.throwErrByPath('breaking_change.blob_util2', {
            args: {
              functionName: key,
            },
          })
        }

Perhaps wrapping it around Cypress.Promise.resolve(...) still invokes the .then method that was added by Cypress?

According to the bluebird docs:

http://bluebirdjs.com/docs/api/promise.resolve.html If value is a thenable (Promise-like object, like those returned by jQuery's $.ajax), returns a trusted Promise that assimilates the state of the thenable.

mduft commented 3 years ago

Maybe @jennifer-shehane can shed some light on this? What else would have to be done to have a working blob-util again?

chrisbreiding commented 3 years ago

We added the .then to try and add a better error message when using the old API, but now it's clear that wasn't a great idea since it gets confused for a 'thenable'.

You should be able to work around it by using Promise.try instead of Promise.resolve:

// before:
Cypress.Promise.resolve(Cypress.Blob.base64StringToBlob(fileContent, mimeType))
// after:
Cypress.Promise.try(() => Cypress.Blob.base64StringToBlob(fileContent, mimeType)))
mduft commented 3 years ago
Cypress.Promise.try(() => Cypress.Blob.base64StringToBlob(fileContent, mimeType)))

I tried with this code instead, but I get the very same result...

base64StringToBlob() no longer returns a Promise. Update the use of base64StringToBlob() to expect a returned Blob.
emilong commented 3 years ago

Thanks, @chrisbreiding! I missed that detail... still a bit early in the morning :sweat_smile: :coffee:

EtienneBruines commented 3 years ago

Cypress.Promise.try does not mean "meh, I don't care if it works" though.

It's just a fancy way of handling the non-async part of the potential errors.

http://bluebirdjs.com/docs/api/promise.try.html Start the chain of promises with Promise.try. Any synchronous exceptions will be turned into rejections on the returned promise.

chrisbreiding commented 3 years ago

That's my bad. I suggested it before trying it out. I had hoped it wouldn't trigger using .then on the return value like Promise.resolve, but it looks like it does.

Might have to resort to an ugly hack to fix this. Something like the following:

const removeThen = (value) => {
  if (value) value.then = undefined

  return value
}

Cypress.Promise.resolve(removeThen(Cypress.Blob.base64StringToBlob(fileContent, mimeType)))
emilong commented 3 years ago

I'm thinking we'll want to condition that on the Cypress version too? To be compatible with < 5.0.0. I'll draft it up.

chrisbreiding commented 3 years ago

Sorry for the hassle with this one. We'll improve this for the next release, but unfortunately the hack will have to remain for users on 5.0.0.

emilong commented 3 years ago

Ok, I actually did due diligence this time and confirmed https://github.com/abramenal/cypress-file-upload/pull/215/commits/efa9358efb41bb3ccadfc0632fe396d082d388b1 works on my tests locally for 4.12.1 and 5.0.0. :crossed_fingers:

mduft commented 3 years ago

Yeah, I can confirm that this fix work perfectly fine now :)

KatieMFritz commented 3 years ago

Is there an estimate for when this might be merged for the plugin? I'm trying to use the PR branch but I'm running into some issues (webpack can't resolve the module).

mduft commented 3 years ago

MaintainerNotAvailableException?

dragonfire1119 commented 3 years ago

Hope this gets merged soon this works perfectly!

KatieMFritz commented 3 years ago

Can anyone help me figure out how to use the https://github.com/binti-family/cypress-file-upload/tree/cypress-v5-compat branch in my build?

What I did:

  1. Delete cypress-file-upload from node_modules directory.
  2. Delete yarn.lock.
  3. Add to devDependencies in package.json: "cypress-file-upload": "binti-family/cypress-file-upload#cypress-v5-compat",
  4. yarn install
  5. Add to cypress/support/commands.js: import 'cypress-file-upload'

When I run my test, I get this error:

rror: Webpack Compilation Error
./cypress/support/commands.js
Module not found: Error: Can't resolve 'cypress-file-upload' in '/[myroot]/cypress/support'
resolve 'cypress-file-upload' in '[myroot]/cypress/support'
  Parsed request is a module
  using description file: [myroot]/package.json (relative path: ./cypress/support)
    Field 'browser' doesn't contain a valid alias configuration
    Looked for and couldn't find the file at the following paths:

When I inspect the import command in my IDE, I see it's linking to node_modules/cypress-file-upload/types/index.d.ts.

I'm using yarn workspaces, so my node_modules directory is a higher up than my package.json where I'm requiring stuff.

=== SOLVED by dragonfire1119!

cd /node_modules/cypress-file-input
yarn
yarn build
dragonfire1119 commented 3 years ago

Can anyone help me figure out how to use the https://github.com/binti-family/cypress-file-upload/tree/cypress-v5-compat branch in my build?

What I did:

  1. Delete cypress-file-upload from node_modules directory.

  2. Delete yarn.lock.

  3. Add to devDependencies in package.json: `"cypress-file-upload": "binti-family/cypress-file-upload#cypress-v5-compat",

`

  1. yarn install

  2. Add to cypress/support/commands.js: import 'cypress-file-upload'

When I run my test, I get this error:


rror: Webpack Compilation Error

./cypress/support/commands.js

Module not found: Error: Can't resolve 'cypress-file-upload' in '/[myroot]/cypress/support'

resolve 'cypress-file-upload' in '[myroot]/cypress/support'

  Parsed request is a module

  using description file: [myroot]/package.json (relative path: ./cypress/support)

    Field 'browser' doesn't contain a valid alias configuration

    Looked for and couldn't find the file at the following paths:

I'm using yarn workspaces, so my node_modules directory is a higher up than my package.json where I'm requiring stuff.

You have to go into the directory and yarn; yarn build. Yarn is not building the dist inside node_modules/cypress-file-upload.

KatieMFritz commented 3 years ago

Thanks, @dragonfire1119 ! That did the trick.

dragonfire1119 commented 3 years ago

Thanks, @dragonfire1119 ! That did the trick.

You're welcome! Glad I could help! :)

emilong commented 3 years ago

@abramenal any chance you have time to check this out and either provide feedback or get a release going? really appreciate your work on this package and just hoping to unblock some folks :blush:

testtek commented 3 years ago

Shall we get this PR sooner to the release build? Blocking my test execution and refraining me from upgrade Cypress@5.0.0

josephzidell commented 3 years ago

Hey all! After speaking with @abramenal, it seems the best way forward is for another person to become a maintainer on this project. Looking forward to working with you all!

This is my first time being an OSS maintainer, so please be gentle.

emilong commented 3 years ago

Ok! Updated to use semver for comparison and I tried it locally on Cypress 4 and 5. Would appreciate anyone else double checking though!

josephzidell commented 3 years ago

@abramenal Before this gets merged, I asked the author to try a different way that introduces no new dependencies and might also be safer. Hopefully we can wait 1 more day for that.

emilong commented 3 years ago

I had a chance to update to use @josephzidell's approach and check it on Cypress 4 and 5 again.

josephzidell commented 3 years ago

LGTM

josephzidell commented 3 years ago

@all-contributors add @emilong for code

allcontributors[bot] commented 3 years ago

@josephzidell

I've put up a pull request to add @emilong! :tada:

jojost1 commented 3 years ago

Will you also make a new release?

josephzidell commented 3 years ago

We're working on it 🍻

abramenal commented 3 years ago

@all-contributors add @josephzidell for maintenance

allcontributors[bot] commented 3 years ago

@abramenal

I've put up a pull request to add @josephzidell! :tada:

abramenal commented 3 years ago

Thanks everyone for working on this, v4.1.0 is out 🎉🎉🎉

josephzidell commented 3 years ago

Unfortunately, Cypress 5 is still showing this error:

binaryStringToBlob() no longer returns a Promise. Update the use of binaryStringToBlob() to expect a returned Blob
testtek commented 3 years ago

Thank you !!! I can confirmed that it worked !! 👍

martinsoenen commented 3 years ago

It worked for me too !! Thanks a lot

bbonevPIT commented 3 years ago

I just pulled the latest code, I can confirm the file upload is working on the latest cypress version, thank you so much guys!

JohnnyChiang commented 3 years ago

Unfortunately, Cypress 5 is still showing this error:

binaryStringToBlob() no longer returns a Promise. Update the use of binaryStringToBlob() to expect a returned Blob

Having the same error here

Ksan8 commented 3 years ago

I'm also still getting the error that @josephzidell and @JohnnyChiang mentioned. Going to downgrade again until this is stable.

emilong commented 3 years ago

@Ksan8 @josephzidell @JohnnyChiang can you say more about your set up? E.g. I'm using cypress to drive chrome only and I didn't have any other plugins/extensions that hit this issue. Those are the first two things that come to mind as being possibilities?

saschwarz commented 3 years ago

@Ksan8 @josephzidell @JohnnyChiang take a look at #222 and #225 to see if that is your use case. Also, 4.1.1 has been released with a fix.