-
-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Transfer web proxy steps to extension-selenium #2256
Transfer web proxy steps to extension-selenium #2256
Conversation
bb9cf30
to
cb7d365
Compare
@@ -32,6 +33,11 @@ project.description = 'Vividus extension for selenium' | |||
implementation(group: 'org.apache.commons', name: 'commons-lang3', version: versions.commonsLang3) | |||
implementation(group: 'org.springframework', name: 'spring-core', version: versions.spring) | |||
|
|||
// browserup-proxy depends on both netty-all and netty-core, so we need to align versions of all netty dependencies | |||
// https://github.com/browserup/browserup-proxy/blob/26714e26fae728733a13a0f332bd0c6dcaa78de5/browserup-proxy-core/build.gradle#L60-L76 | |||
implementation(group: 'io.netty', name: 'netty-all', version: "${nettyVersion}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we write version as for seleniumVersion
or browserupProxyVersion
?
import org.openqa.selenium.WebDriver; | ||
import org.vividus.ui.action.IWaitActions; | ||
|
||
public interface IWebWaitActions extends IWaitActions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it shouldn't be moved
.Validate the URL query of the matching HTTP request | ||
[source,gherkin] | ||
---- | ||
Given I am on a page with the URL 'https://www.google.com/search?q=vividus' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these examples are not applicable for mobile apps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{givent-step}
@@ -744,3 +746,5 @@ When I select next value with `0.1` offset in picker wheel located `xpath(//XCUI | |||
|
|||
|
|||
include::partial$generic-ui-steps.adoc[] | |||
|
|||
include::partial$proxy-steps.adoc[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should put some extra info, that proxy is not supported by Appium and custom solutions are needed
Codecov Report
@@ Coverage Diff @@
## master #2256 +/- ##
============================================
+ Coverage 87.99% 96.30% +8.30%
+ Complexity 5528 5363 -165
============================================
Files 762 762
Lines 15372 15408 +36
Branches 1024 1024
============================================
+ Hits 13527 14838 +1311
+ Misses 1713 445 -1268
+ Partials 132 125 -7
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also please make sure the corresponding unit tests are moved as well
|=== | ||
|
||
include::partial$proxy-meta-tags.adoc[] | ||
|
||
IMPORTANT: For device configuration, we need to perform the following steps: install CA certificate(download CA certificate from a browserup repository), configure device proxy, and configure the test app to allow proxy in cases of necessity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add a link to CA certificate to download
|=== | ||
|
||
include::partial$proxy-meta-tags.adoc[] | ||
|
||
IMPORTANT: For device configuration, we need to perform the following steps: install CA certificate(download CA certificate from a browserup repository), configure device proxy, and configure the test app to allow proxy in cases of necessity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please elaborate on "configure device proxy"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean the following steps:
- go to the device’s Settings and Wifi configuration
- press on the network you’re going to use
- tap a Proxy field and select Manual
- add a proxy hostname and a proxy port
@@ -1,5 +1,7 @@ | |||
:grid-default-hostname: empty | |||
:keys: https://github.com/appium/appium-xcuitest-driver#mobile-pressbutton[iOS keys] or https://appium.github.io/java-client/io/appium/java_client/android/nativekey/AndroidKey.html[Android keys] | |||
:proxy: This step requires proxy to be turned on. It can be done in properties or by switching on @proxy meta tag at the story level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can it be a part of proxy-steps.adoc
?
91f794b
to
ac4cfe6
Compare
|Overrides the host which will be used by proxy | ||
|
||
|proxy.ports | ||
|10000-10005 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't it be any port from not reserved/registered range i.e. 49152 to 65535?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just wondering why 10000-10005
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve conflicts at first
…ork#2252) Bumps [vividus-build-system](https://github.com/vividus-framework/vividus-build-system) from `7850e3c` to `ca2a0c3`. - [Release notes](https://github.com/vividus-framework/vividus-build-system/releases) - [Commits](vividus-framework/vividus-build-system@7850e3c...ca2a0c3) --- updated-dependencies: - dependency-name: vividus-build-system dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
6459c43
to
4c03bea
Compare
.github/workflows/gradle.yml
Outdated
-Pvividus.bdd.variables.global.app-url=sauce-storage:vividus-test-app-ios.zip \ | ||
-Pvividus.statistics.print-failures=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-Pvividus.bdd.variables.global.app-url=sauce-storage:vividus-test-app-ios.zip \ | |
-Pvividus.statistics.print-failures=true | |
-Pvividus.bdd.variables.global.app-url=sauce-storage:vividus-test-app-ios.zip |
.github/workflows/gradle.yml
Outdated
-Pvividus.bdd.variables.global.app-url=sauce-storage:vividus-test-app-android.zip \ | ||
-Pvividus.statistics.print-failures=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-Pvividus.bdd.variables.global.app-url=sauce-storage:vividus-test-app-android.zip \ | |
-Pvividus.statistics.print-failures=true | |
-Pvividus.bdd.variables.global.app-url=sauce-storage:vividus-test-app-android.zip |
@@ -3,6 +3,7 @@ project.description = 'Vividus extension for selenium' | |||
ext { | |||
seleniumVersion = '4.1.1' | |||
browserupProxyVersion = '2.1.2' | |||
nettyVersion = '4.1.70.Final' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also should the same dependencies be removed from vividus-plugin-web-app
?
.github/workflows/gradle.yml
Outdated
./gradlew :vividus-tests:debugStories -Pvividus.configuration.environments=system/saucelabs/mobile_app/ios \ | ||
-Pvividus.configuration.suites= \ | ||
-Pvividus.configuration.profiles=saucelabs/mobile_app,mobile_app/ios \ | ||
-Pvividus.bdd.story-loader.batch-1.resource-location=story/integration \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the story location is incorrect
.github/workflows/gradle.yml
Outdated
./gradlew :vividus-tests:debugStories -Pvividus.configuration.environments=system/saucelabs/mobile_app/android \ | ||
-Pvividus.configuration.suites= \ | ||
-Pvividus.configuration.profiles=saucelabs/mobile_app,mobile_app/android \ | ||
-Pvividus.bdd.story-loader.batch-1.resource-location=story/integration \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a83af3e
to
e614a64
Compare
No description provided.