Skip to content
This repository has been archived by the owner on Jul 29, 2024. It is now read-only.

chore(test): move element_spec off of the control flow #4998

Closed
wants to merge 1 commit into from
Closed

chore(test): move element_spec off of the control flow #4998

wants to merge 1 commit into from

Conversation

CrispusDH
Copy link
Contributor

@CrispusDH CrispusDH commented Nov 7, 2018

  • note that JSHint still does not support async ... await - Support for async functions jshint/jshint#2604 . It might be better to move to ESLint
  • add try ... catch
  • add await
  • all tests were passed locally using directConnect: true
  • I get flaky EPIPE errors for .filter(), .map(), .reduce() methods - there are several issues that related to it (there are related to concurrent requests issues, not for async ... await

* note that JSHint still does not support `async ... await` - jshint/jshint#2604 . It might be better to move to ESLint
* add `try ... catch`
* add `await`
* I get flaky EPIPE errors for .filter(), .map(), .reduce() methods
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

1 similar comment
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@CrispusDH
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

1 similar comment
@googlebot
Copy link

CLAs look good, thanks!

@cnishina
Copy link
Member

cnishina commented Nov 7, 2018

Thanks for this. I found some issues with my work. This was a great commit either way. If you want to help out, I'm going to work on:

  • locators_spec.js
  • lib_spec.js
  • mockmodule_spec.js

and then I'll jump around to other tests in the suite.

This means in the basic, I will not touch these files if you want to take a shot at them. Please work on one file at a time as a pull request.

  • expected_conditions_spec.js
  • handling_spec.js
  • navigation_spec.js
  • polling_spec.js
  • restart_spec.js
  • synchronize_spec.js

Also a quick review of the PR submitted, it looks like you have a lot of trailing spaces on lines. If you are using visual studio code, there is a setting to remove trailing settings. (https://stackoverflow.com/questions/30884131/remove-trailing-spaces-automatically-or-with-a-shortcut)

I'll also add this to the tracking issue. Thanks again!

@cnishina cnishina closed this Nov 7, 2018
@cnishina cnishina mentioned this pull request Nov 7, 2018
19 tasks
@CrispusDH CrispusDH deleted the move-element-spec-off-cf branch November 7, 2018 21:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants