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

remove hardcoded implicitlywait #82

Merged
merged 1 commit into from
Jun 2, 2015
Merged

remove hardcoded implicitlywait #82

merged 1 commit into from
Jun 2, 2015

Conversation

emaks
Copy link
Contributor

@emaks emaks commented May 11, 2015

pull request for issue #78


public static int getTimeOutInSeconds() {
try {
return Integer.valueOf(System.getProperty("webdriver.timeouts.implicitlywait"));
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great if this can be overridden on per-element level using a @Timeout annotation for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Timeout annotation have been added

@emaks
Copy link
Contributor Author

emaks commented May 26, 2015

@artkoshelev is there any updates?

@artkoshelev
Copy link
Contributor

test this please

} catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
}

return 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

we should return 5 here to keep backward compatibility with previous implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should return 5 here to keep backward compatibility with previous implementation

in that case I have to add annotations for all elements in my project for changing value less than 5 seconds. It's redundant for me 👎

Copy link
Contributor

Choose a reason for hiding this comment

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

can you suggest any solution to this problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be this solution good for you?

private int getDefaultTimeOut() {
        try {
            return Integer.valueOf(System.getProperty("webdriver.timeouts.implicitlywait"));
        } catch (NumberFormatException ex) {
            return 5;
        }
    }

Copy link
Contributor

Choose a reason for hiding this comment

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

There is default method to get integer from property with default value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean this case?

Integer.getInteger(System.getProperty("webdriver.timeouts.implicitlywait"), 5);

Copy link
Contributor

Choose a reason for hiding this comment

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

Just Integer.getInteger("webdriver.timeouts.implicitlywait", 5);

@eroshenkoam
Copy link
Contributor

test this please


public int getTimeOut(Field field) {
if (field.isAnnotationPresent(Timeout.class)) {
return field.getDeclaredAnnotation(Timeout.class).value();
Copy link
Contributor

Choose a reason for hiding this comment

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

getDelcaredAnnotation(Class) method is not available in java7, please refactor this place to keep java7-compatible

@emaks
Copy link
Contributor Author

emaks commented May 28, 2015

test this please

@eroshenkoam @artkoshelev
Why these tests are not enough?

@artkoshelev
Copy link
Contributor

@emaks don't worry about 'test this please' it's just a phrase to run jenkins build

@emaks
Copy link
Contributor Author

emaks commented Jun 2, 2015

@artkoshelev
Are there any new issues that must be resolved?

@lanwen
Copy link
Contributor

lanwen commented Jun 2, 2015

@emaks Please add test with system property usage

@emaks
Copy link
Contributor Author

emaks commented Jun 2, 2015

Please add test with system property usage

done

@lanwen
Copy link
Contributor

lanwen commented Jun 2, 2015

LGTM

artkoshelev added a commit that referenced this pull request Jun 2, 2015
remove hardcoded implicitlywait
@artkoshelev artkoshelev merged commit 51eba9c into yandex-qatools:master Jun 2, 2015
@artkoshelev
Copy link
Contributor

good job, landed!

@emaks
Copy link
Contributor Author

emaks commented Jun 4, 2015

@artkoshelev Could you, please, tell me when next release will be?

@emaks emaks deleted the master branch June 4, 2015 13:10
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 this pull request may close these issues.

5 participants