Skip to content

Conversation

@STRd6
Copy link
Contributor

@STRd6 STRd6 commented Apr 5, 2022

When running browser tests sometimes they would fail and only print null. I added a .catch to log any error that is thrown (could be a timeout or intermittent error). I also removed extra parentheses and increased the timeout to 60s.

@GeoffreyBooth
Copy link
Collaborator

 # and Puppeteer 3 only supports Node >= 10.18.1, so limit this test to those # versions. The code below uses `Promise.prototype.finally` because the # CoffeeScript codebase currently maintains compatibility with Node 6, which # did not support `async`/`await` syntax. Even though this test doesn’t run # in Node 6, it needs to still _parse_ in Node 6 so that this file can load.

This was true when I wrote it years ago, but I think that by now we can drop support for EOL versions of Node at least for internal stuff like running tests. (The compiled output, I’m not so sure about.) So we could just rewrite this function to use await, if you want. Also Puppeteer has been updated many times since this comment was written and now requires something like Node 12+.

Anyway I’m happy to merge in this PR as is, too, if you’d rather not spend any more time on it.

Removed comment and check for older versions of Node as we no longer support them anymore.
@GeoffreyBooth
Copy link
Collaborator

FYI: https://github.com/jashkenas/coffeescript/runs/5842054989?check_suite_focus=true

Should this have reported something other than null?

@STRd6
Copy link
Contributor Author

STRd6 commented Apr 5, 2022

It should probably report nothing in the case where result hasn't been filled. I'll add that.

@GeoffreyBooth GeoffreyBooth merged commit 76cf769 into jashkenas:main Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants