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

[hotfix][e2e] refactor jupiter,add junit-jupiter-api #2662

Merged
merged 10 commits into from
Sep 8, 2022

Conversation

liugddx
Copy link
Member

@liugddx liugddx commented Sep 6, 2022

Purpose of this pull request

refactor jupiter,add junit-jupiter-api

Check list

@TyrantLucifer
Copy link
Member

Can you explain why you want to add this dependency?

@CalvinKirs
Copy link
Member

the e2e module used junit 4.

@liugddx
Copy link
Member Author

liugddx commented Sep 6, 2022

Occasionally switching branches leads to inconsistent versions of api dependencies
ecfb805b4e601305b5f182eee5db365

Copy link
Member

@CalvinKirs CalvinKirs left a comment

Choose a reason for hiding this comment

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

I think we have declared these dependencies, junit-jupiter-params inculde juitper-api
if you checked. e2e We use junit4 and 5 for Testcontainers reasons

@liugddx
Copy link
Member Author

liugddx commented Sep 7, 2022

I think we have declared these dependencies, junit-jupiter-params inculde juitper-api if you checked. e2e We use junit4 and 5 for Testcontainers reasons

but the e2e dependence jupiter.if the junit-jupiter-api has a different versions.the error will happen.

pom.xml Outdated Show resolved Hide resolved
@ashulin
Copy link
Member

ashulin commented Sep 7, 2022

image
In which dependency will junit-jupiter-api-5.8.2 be referenced?

@liugddx
Copy link
Member Author

liugddx commented Sep 7, 2022

image In which dependency will junit-jupiter-api-5.8.2 be referenced?

i dependence spring-boot-dependencies,and the junit-jupiter.versionis 5.8.2.

@ashulin ashulin added bug and removed invalid labels Sep 7, 2022
@ashulin
Copy link
Member

ashulin commented Sep 7, 2022

This PR contains the reason: #2672 . I ran CI again.

@CalvinKirs
Copy link
Member

hi, we better revert https://github.com/apache/incubator-seatunnel/blob/34ccfd7a2cd93d8dc0d3f6bb1e1afebcc9761a9e/pom.xml#L360
This modification changes so many dependencies that I suspect there are still a lot of potential issues that haven't been discovered, but as long as we revert it, this issue and other issues can be resolved.

@CalvinKirs
Copy link
Member

image
revert this change

Hisoka-X
Hisoka-X previously approved these changes Sep 7, 2022
@zhuangchong
Copy link
Contributor

zhuangchong commented Sep 8, 2022

My suggestion is to introduce junit-bom for dependency management and remove junit-jupiter-engine and junit-jupiter-params with the following changes:

add:

  <dependency>
      <groupId>org.junit</groupId>
      <artifactId>junit-bom</artifactId>
      <version>${junit.version}</version>
      <type>pom</type>
      <scope>import</scope>
  </dependency>

remote:

<dependency>
      <groupId>org.junit.jupiter</groupId>
      <artifactId>junit-jupiter-engine</artifactId>
      <version>${junit.version}</version>
      <scope>test</scope>
  </dependency>

  <dependency>
      <groupId>org.junit.jupiter</groupId>
      <artifactId>junit-jupiter-params</artifactId>
      <version>${junit.version}</version>
      <scope>test</scope>
  </dependency>

update:

<dependencies>
...
 <dependency>
            <groupId>org.junit.jupiter</groupId>
            <artifactId>junit-jupiter-engine</artifactId>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>org.junit.jupiter</groupId>
            <artifactId>junit-jupiter-params</artifactId>
            <scope>test</scope>
        </dependency>

    </dependencies>

@zhuangchong
Copy link
Contributor

UserControllerTest executes an error after the scheduler function is added to the web module, because the web module needs to connect to the dolphinscheduler server, I will deal with this problem.

pom.xml Outdated Show resolved Hide resolved
@CalvinKirs CalvinKirs merged commit ad43f14 into apache:dev Sep 8, 2022
@liugddx liugddx deleted the hotfix-junitdependency branch September 8, 2022 12:07
MRYOG pushed a commit to MRYOG/incubator-seatunnel that referenced this pull request Sep 8, 2022
Co-authored-by: CalvinKirs <kirs@apache.org>
Co-authored-by: Zongwen Li <zongwen.li.tech@gmail.com>
laglangyue pushed a commit to laglangyue/seatunnel that referenced this pull request Sep 11, 2022
Co-authored-by: CalvinKirs <kirs@apache.org>
Co-authored-by: Zongwen Li <zongwen.li.tech@gmail.com>
lhyundeadsoul pushed a commit to lhyundeadsoul/incubator-seatunnel that referenced this pull request Sep 13, 2022
Co-authored-by: CalvinKirs <kirs@apache.org>
Co-authored-by: Zongwen Li <zongwen.li.tech@gmail.com>
MRYOG pushed a commit to MRYOG/incubator-seatunnel that referenced this pull request Sep 15, 2022
Co-authored-by: CalvinKirs <kirs@apache.org>
Co-authored-by: Zongwen Li <zongwen.li.tech@gmail.com>
MRYOG pushed a commit to MRYOG/incubator-seatunnel that referenced this pull request Sep 16, 2022
Co-authored-by: CalvinKirs <kirs@apache.org>
Co-authored-by: Zongwen Li <zongwen.li.tech@gmail.com>
TyrantLucifer pushed a commit to TyrantLucifer/incubator-seatunnel that referenced this pull request Sep 18, 2022
Co-authored-by: CalvinKirs <kirs@apache.org>
Co-authored-by: Zongwen Li <zongwen.li.tech@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants