Skip to content

[SPARK-40397][BUILD] Upgrade org.scalatestplus:selenium to 3.12.13 #37868

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.

Sign up for GitHub

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

Closed
LuciferYang wants to merge 5 commits into from
Closed

[SPARK-40397][BUILD] Upgrade org.scalatestplus:selenium to 3.12.13 #37868

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c67a3a1
for test
LuciferYang Sep 13, 2022
e6c149d
upgrade okio
LuciferYang Sep 13, 2022
44ba081
htmlunit-driver.version
LuciferYang Sep 13, 2022
d42308e
remove todo
LuciferYang Sep 13, 2022
8c13e52
address comments
LuciferYang Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,11 @@
<artifactId>selenium-java</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>selenium-remote-driver</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>htmlunit-driver</artifactId>
Expand Down
2 changes: 1 addition & 1 deletion dev/deps/spark-deps-hadoop-2-hive-2.3
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ netty-transport-native-unix-common/4.1.80.Final//netty-transport-native-unix-com
netty-transport/4.1.80.Final//netty-transport-4.1.80.Final.jar
objenesis/3.2//objenesis-3.2.jar
okhttp/3.12.12//okhttp-3.12.12.jar
okio/1.14.0//okio-1.14.0.jar
okio/1.15.0//okio-1.15.0.jar
Copy link
Contributor Author

@LuciferYang LuciferYang Sep 13, 2022

Choose a reason for hiding this comment

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

The reason for the change is explained in #37867

Copy link
Contributor Author

@LuciferYang LuciferYang Sep 13, 2022

Choose a reason for hiding this comment

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

When using selenium 3.1, Maven chose nearby okio 1.14.0., both selenium 4.2 and k8s-client rely on 1.15.0 now

opencsv/2.3//opencsv-2.3.jar
orc-core/1.8.0/shaded-protobuf/orc-core-1.8.0-shaded-protobuf.jar
orc-mapreduce/1.8.0/shaded-protobuf/orc-mapreduce-1.8.0-shaded-protobuf.jar
Expand Down
2 changes: 1 addition & 1 deletion dev/deps/spark-deps-hadoop-3-hive-2.3
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ netty-transport-native-unix-common/4.1.80.Final//netty-transport-native-unix-com
netty-transport/4.1.80.Final//netty-transport-4.1.80.Final.jar
objenesis/3.2//objenesis-3.2.jar
okhttp/3.12.12//okhttp-3.12.12.jar
okio/1.14.0//okio-1.14.0.jar
okio/1.15.0//okio-1.15.0.jar
opencsv/2.3//opencsv-2.3.jar
opentracing-api/0.33.0//opentracing-api-0.33.0.jar
opentracing-noop/0.33.0//opentracing-noop-0.33.0.jar
Expand Down
26 changes: 19 additions & 7 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@
<!-- Please don't upgrade the version to 4.10+, it depends on JDK 11 -->
<antlr4.version>4.9.3</antlr4.version>
<jpam.version>1.1</jpam.version>
<selenium.version>3.141.59</selenium.version>
<selenium.version>4.2.2</selenium.version>
<htmlunit-driver.version>3.62.0</htmlunit-driver.version>
Copy link
Member

@srowen srowen Sep 14, 2022

Choose a reason for hiding this comment

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

Huh, so we're using two different versions of different htmlunit components? what if all of htmlunit is updated?

Copy link
Contributor Author

@LuciferYang LuciferYang Sep 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

No, htmlunit-driver is in SeleniumHQ, the latest version is 3.64.0, htmlUnit is in HtmlUnit, and the latest version is 2.64.0

Copy link
Member

@srowen srowen Sep 14, 2022

Choose a reason for hiding this comment

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

Got it, and can it match the selenium version then or are they released pretty differently?

Copy link
Contributor Author

@LuciferYang LuciferYang Sep 14, 2022

Choose a reason for hiding this comment

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

This is a different project maintained by two organizations, the major versions are different

Copy link
Contributor Author

@LuciferYang LuciferYang Sep 14, 2022

Choose a reason for hiding this comment

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

Yes, they match the version of selenium

<htmlunit.version>2.62.0</htmlunit.version>
<maven-antrun.version>1.8</maven-antrun.version>
<commons-crypto.version>1.1.0</commons-crypto.version>
Expand Down Expand Up @@ -408,7 +409,7 @@
</dependency>
<dependency>
<groupId>org.scalatestplus</groupId>
<artifactId>selenium-3-141_${scala.binary.version}</artifactId>
<artifactId>selenium-4-2_${scala.binary.version}</artifactId>
<scope>test</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -688,14 +689,26 @@
<artifactId>selenium-java</artifactId>
<version>${selenium.version}</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>selenium-remote-driver</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>selenium-remote-driver</artifactId>
Copy link
Contributor Author

@LuciferYang LuciferYang Sep 13, 2022

Choose a reason for hiding this comment

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

Explicit management selenium-remote-driver because it requires exclude netty, guava, bytebuddy as before

Copy link
Member

@sarutak sarutak Sep 14, 2022

Choose a reason for hiding this comment

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

Why not just only exclude netty, guava, and bytebuddy rather than selenium-remote-driver?

Copy link
Contributor Author

@LuciferYang LuciferYang Sep 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

When using selenium 3.1, netty, guava, and bytebuddy is the direct dependency of selenium-java, so we can directly exclude them.

But after migrating to selenium 4.2 , netty, guava, and bytebuddy is directly dependency of selenium-remote-driver, not selenium-java.

Copy link
Member

@sarutak sarutak Sep 14, 2022

Choose a reason for hiding this comment

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

Hmm, I think we can exclude indirect dependencies.
I confirmed with build/mvn -Phive -Phive-thriftserver -DskipTests dependency:tree.

jackson-module-scala also excludes an indirect dependency.

spark/pom.xml

Lines 1001 to 1011 in a2304e4

<dependency>
<groupId>com.fasterxml.jackson.module</groupId>
<artifactId>jackson-module-scala_${scala.binary.version}</artifactId>
<version>${fasterxml.jackson.version}</version>
<exclusions>
<exclusion>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</exclusion>
</exclusions>
</dependency>

Copy link
Contributor Author

@LuciferYang LuciferYang Sep 14, 2022

Choose a reason for hiding this comment

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

Amazing!!! Let me have a try

Copy link
Contributor Author

@LuciferYang LuciferYang Sep 14, 2022

Choose a reason for hiding this comment

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

like

<dependency>
        <groupId>org.seleniumhq.selenium</groupId>
        <artifactId>selenium-java</artifactId>
        <version>${selenium.version}</version>
        <scope>test</scope>
        <exclusions>
          <exclusion>
            <groupId>com.google.guava</groupId>
            <artifactId>guava</artifactId>
          </exclusion>
          <exclusion>
            <groupId>io.netty</groupId>
            <artifactId>*</artifactId>
          </exclusion>
          <exclusion>
            <groupId>net.bytebuddy</groupId>
            <artifactId>byte-buddy</artifactId>
          </exclusion>
        </exclusions>
      </dependency>

?

Copy link
Member

@sarutak sarutak Sep 14, 2022

Choose a reason for hiding this comment

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

Yes. But as you noticed, we may need some additional exclusions.

<version>${selenium.version}</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</exclusion>
<exclusion>
<groupId>io.netty</groupId>
<artifactId>netty</artifactId>
<artifactId>*</artifactId>
</exclusion>
<exclusion>
<groupId>net.bytebuddy</groupId>
Expand All @@ -706,7 +719,7 @@
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>htmlunit-driver</artifactId>
<version>${htmlunit.version}</version>
<version>${htmlunit-driver.version}</version>
<scope>test</scope>
</dependency>
<!-- Update htmlunit dependency that selenium uses for better JS support -->
Expand Down Expand Up @@ -1160,11 +1173,10 @@
<version>3.2.13.0</version>
<scope>test</scope>
</dependency>
<!-- TODO(SPARK-40397): Migrate selenium-java from 3.1 to 4.2 and upgrade this to 3.2.13.0 -->
<dependency>
<groupId>org.scalatestplus</groupId>
<artifactId>selenium-3-141_${scala.binary.version}</artifactId>
<version>3.2.10.0</version>
<artifactId>selenium-4-2_${scala.binary.version}</artifactId>
<version>3.2.13.0</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down
5 changes: 5 additions & 0 deletions sql/core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,11 @@
<artifactId>selenium-java</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>selenium-remote-driver</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>htmlunit-driver</artifactId>
Expand Down
5 changes: 5 additions & 0 deletions sql/hive-thriftserver/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@
<artifactId>selenium-java</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>selenium-remote-driver</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>htmlunit-driver</artifactId>
Expand Down
5 changes: 5 additions & 0 deletions streaming/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@
<artifactId>selenium-java</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>selenium-remote-driver</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>htmlunit-driver</artifactId>
Expand Down