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

run-browser is an unmaintained project #180

Closed
ossdev07 opened this issue Apr 24, 2019 · 14 comments · Fixed by naugtur/run-browser#1
Closed

run-browser is an unmaintained project #180

ossdev07 opened this issue Apr 24, 2019 · 14 comments · Fixed by naugtur/run-browser#1

Comments

@ossdev07
Copy link
Contributor

Hello Team,

The dependency run-browser used by the xhr for testing the package is now unmaintained.

Further, the browser dependency used by run-browser is also out of date i.e the phantomjs browser.

I am curious to know that if anyone here has considered using a maintained alternative to run-browser for xhr.

Please do share your thoughts on this.

@naugtur
Copy link
Owner

naugtur commented Apr 24, 2019

There's an issue where we were discussing setting it up on browserstack or similar. It was a non critical task and time consuming but not particularly demanding, so it was waiting for a volunteer.

I'd follow through with that.

But please share your thoughts if you have other ideas.

@Raynos
Copy link
Collaborator

Raynos commented Apr 24, 2019

Don’t fix what is not broken.

@ossdev07
Copy link
Contributor Author

ossdev07 commented Oct 1, 2019

@naugtur Will, you be interested in accepting a PR of PhantomJS replacement with puppeteer?

@naugtur
Copy link
Owner

naugtur commented Oct 3, 2019

I would - to avoid pulling PhantomJS binary on npm install.
Especially if it somehow helped in moving this forward: #81

@ossdev07
Copy link
Contributor Author

ossdev07 commented Nov 6, 2019

Hi @naugtur, I have successfully replaced PhantomJS with puppeteer, but not able raise pull request by forking https://github.com/naugtur/run-browser. As it is redirecting to the another run-browser which seems unmaintained .

Could you please let me know how can I ship the changes so that you can merge it to be used by xhr.

@Raynos
Copy link
Collaborator

Raynos commented Nov 7, 2019

Puppeteer is a good project. Rewriting the tests to leverage puppeteer directly seems reasonable.

@ossdev07
Copy link
Contributor Author

ossdev07 commented Nov 7, 2019

Thanks for replying @Raynos I have sucessfully used puppeteer in xhr by modiying the run-browser, and all the test cases are passing, Please have a look at the logs:

x64_server@x_server_embedded:~/NodeJS/test/xhr$ npm test> [email protected] test /home/x64_server/NodeJS/test/xhr
> run-browser test/index.js -b -m test/mock-server.js | tap-spec
  constructs and calls callback without throwing    Failed to load resource: the server responded with a status of 404 (Not Found)
    ✔ got here  [func] Can GET a url (cross-domain)    ✔ no err
    ✔ should be equal
    ✔ should be equal
    ✔ should not be equal
    ✔ should be equal
    ✔ should not be equal  [func] Returns http error responses like npm's request (cross-domain)    Failed to load resource: the server responded with a status of 404 (Not Found)
    ✔ no err
    ✔ should be equal
    ✔ should be equal  [func] Request to domain with not allowed cross-domain    Failed to load resource: the server responded with a status of 500 (Internal Server Error)
    ✔ should be truthy
    ✔ should be equal
    ✔ should be equal  [func] Returns a falsy body for 2xx responses    mock: /mock/no-content/200
    mock: /mock/no-content/204
    mock: /mock/no-content/201
    mock: /mock/no-content/205
    ✔ body should be falsy
    ✔ should be equal
    ✔ body should be falsy
    ✔ should be equal
    ✔ body should be falsy
    ✔ should be equal
    ✔ body should be falsy
    ✔ should be equal  [func] Calls the callback at most once even if error is thrown issue #127    Access to XMLHttpRequest at 'instanterror://foo' from origin 'http://localhost:3000' has been blocked by CORS policy: Cross origin requests are only supported for protocol schemes: http, data, chrome, https.
    Uncaught Error: dummy error
    http://localhost:3000/tests-bundle.js:12584
    ✔ expected at most one call  [func] Times out to an error     mock: /mock/timeout
    ✔ should return error
    ✔ should be equal
    ✔ should be equal
    ✔ should be equal  withCredentials option    ✔ withCredentials not true
    ✔ withCredentials set to true  withCredentials ignored when using synchronous requests    Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check https://xhr.spec.whatwg.org/.
    Failed to load resource: the server responded with a status of 404 (Not Found)
    ✔ sync overrides withCredentials  XDR usage (run on IE8 or 9)    ✔ Uses XDR when told to  handles errorFunc call with no arguments provided    ✔ callback should get an error
    ✔ error message incorrect
    ✔ should not throw when error handler called without arguments  constructs and calls callback without throwing    ✔ callback is not optional  [func] xhr[method] get, put, post, patch    mock: /mock/200ok
    mock: /mock/200ok
    Failed to load resource: the server responded with a status of 404 (Not Found)
    Failed to load resource: the server responded with a status of 404 (Not Found)
    mock: /mock/200ok
    mock: /mock/200ok
    Failed to load resource: the server responded with a status of 404 (Not Found)
    ✔ no err
    ✔ should be equal
    ✔ should be equal
    ✔ no err
    ✔ should be equal
    ✔ should be equal
    ✔ no err
    ✔ should be equal
    ✔ should be equal
    ✔ no err
    ✔ should be equal
    ✔ should be equal  xhr[method] get, put, post, patch with url shorthands    ✔ should be equal
    ✔ should be equal
    ✔ should be equal
    ✔ should be equal  [func] sends options.body as json body when options.json === true    Failed to load resource: the server responded with a status of 404 (Not Found)
    mock: /mock/echo
    Failed to load resource: the server responded with a status of 404 (Not Found)
    Failed to load resource: the server responded with a status of 404 (Not Found)
    Failed to load resource: the server responded with a status of 404 (Not Found)
    ✔ should be equal
    ✔ should be equivalent  [func] doesn't freak out when json option is false    mock: /mock/echo
    ✔ should not be equal
    ✔ should be equal  [func] sends options.json as body when it's not a boolean    mock: /mock/echo
    ✔ should be equal
    ✔ should be equivalent  xhr[method] get, put, post, patch with url shorthands and options    Failed to load resource: the server responded with a status of 404 (Not Found)
    ✔ should be equal
    ✔ should be equal
    Failed to load resource: the server responded with a status of 404 (Not Found)
    ✔ should be equal
    ✔ should be equal
    Failed to load resource: the server responded with a status of 404 (Not Found)
    ✔ should be equal
    ✔ should be equal
    Failed to load resource: the server responded with a status of 404 (Not Found)
    ✔ should be equal
    ✔ should be equal  [func] xhr.head    mock: /mock/200ok
    ✔ no err
    ✔ should be equal
    ✔ should be equal
    ✔ should be falsy  xhr.head url shorthand    mock: /mock/200ok
    ✔ should be equal  [func] xhr.del    mock: /mock/200ok
    ✔ no err
    ✔ should be equal
    ✔ should be equal  xhr.del url shorthand    mock: /mock/200ok
    ✔ should be equal  url signature without object    Failed to load resource: the server responded with a status of 404 (Not Found)
    ✔ should be equal  url signature with object    Failed to load resource: the server responded with a status of 404 (Not Found)
    ✔ should be equal
    ✔ should be equal  aborting XHR immediately prevents callback from being called
  aborting XHR asynchronously still prevents callback from being called    mock: /mock/timeout  XHR can be overridden    ✔ created the custom XHR
    ✔ created the custom XDR
  total:     78
  passing:   78
  duration:  4.1s

Please do revert what else is required to be done from my end.

@naugtur
Copy link
Owner

naugtur commented Nov 7, 2019

Where can we look at your changes? At this point, if there's significant work on top of Puppeteer in there I suggest you release it under a new name (other than run-browser). I can help you out in the process.
Where can we look at your work?
If it's mostly just calling puppeteer, we could bring all the necessary bits into the test setup instead. That's the thing to figure out here.

@ossdev07
Copy link
Contributor Author

ossdev07 commented Nov 8, 2019

@naugtur These are the files which are modified in order to replace PhantomJS with puppeteer:

In run-browser

  • Modified Readme.md, cli.js, index.js, package.json

  • Deleted phantom-function-bind-shim.js, phantom-script.js, run-phantom.js

In xhr

  • Modified test/index.js

Where can we look at your changes?

I am ready with the PR but not sure about the process to share the changes in your repository.

I can help you out in the process

Yes, please share the process through which I can share the changes with you, if you agree I can share the changes in your repository itself https://github.com/naugtur/run-browser as I have only called puppeteer and the changes are very small.

@naugtur
Copy link
Owner

naugtur commented Nov 9, 2019

Before we proceed - how difficult in your opinion would it be to use pupeteer directly in xhr for tests to eliminate run-browser entirely?

@ossdev07
Copy link
Contributor Author

@naugtur I will look into it and get back to you shortly.

@ossdev07
Copy link
Contributor Author

@naugtur

Before we proceed - how difficult in your opinion would it be to use pupeteer directly in xhr for tests to eliminate run-browser entirely?

I have done the analysis and looks like removing run-browser entirely will be a bigger task. In my view we should use run-browser.

Please let me know your opinion on the same.

@naugtur
Copy link
Owner

naugtur commented Nov 19, 2019

I appreciate any contribution and while I think skipping run-browser would be simpler license-wise and more future proof, forking run-browser under a new name with puppeteer underneath is also a great move.

Please share a link to your work or create a PR. I'll do my best to help out with releasing your work. Under your name, obviously. Let me know your opinions on that (to be clear, I'm not a maintainer of run-browser, I also forked it)

@ossdev07
Copy link
Contributor Author

@naugtur I have raised the pull request with the puppeteer changes in https://github.com/naugtur/run-browser, Please have a look and share your feedback.

Also once you merge the pull request of run-browser naugtur/run-browser#1, I will raise another pull request for xhr as it will result in Travis-ci failures before it.

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 a pull request may close this issue.

3 participants