Skip to content

Conversation

AlbumenJ
Copy link
Member

What is the purpose of the change

Add Github Action Support for Dubbo Unit Test

@codecov-commenter
Copy link

Codecov Report

Merging #6726 into master will increase coverage by 0.22%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #6726      +/-   ##
============================================
+ Coverage     58.84%   59.07%   +0.22%     
+ Complexity      508      507       -1     
============================================
  Files          1025     1025              
  Lines         41341    41341              
  Branches       6014     6014              
============================================
+ Hits          24326    24421      +95     
+ Misses        14262    14187      -75     
+ Partials       2753     2733      -20     
Impacted Files Coverage Δ Complexity Δ
...rg/apache/dubbo/remoting/utils/PayloadDropper.java 38.46% <0.00%> (-7.70%) 0.00% <0.00%> (ø%)
...e/dubbo/remoting/transport/netty/NettyChannel.java 52.27% <0.00%> (-3.41%) 19.00% <0.00%> (-1.00%)
...rg/apache/dubbo/common/timer/HashedWheelTimer.java 63.44% <0.00%> (ø) 0.00% <0.00%> (ø%)
.../apache/dubbo/config/bootstrap/DubboBootstrap.java 42.85% <0.00%> (+0.16%) 0.00% <0.00%> (ø%)
...apache/dubbo/common/extension/ExtensionLoader.java 75.56% <0.00%> (+0.22%) 0.00% <0.00%> (ø%)
...n/java/org/apache/dubbo/common/utils/NetUtils.java 66.55% <0.00%> (+0.34%) 0.00% <0.00%> (ø%)
...ava/org/apache/dubbo/config/ServiceConfigBase.java 57.80% <0.00%> (+0.57%) 0.00% <0.00%> (ø%)
...he/dubbo/registry/multicast/MulticastRegistry.java 68.05% <0.00%> (+0.92%) 0.00% <0.00%> (ø%)
.../dubbo/remoting/transport/netty4/NettyChannel.java 67.32% <0.00%> (+0.99%) 0.00% <0.00%> (ø%)
...a/org/apache/dubbo/rpc/filter/AccessLogFilter.java 81.33% <0.00%> (+1.33%) 0.00% <0.00%> (ø%)
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a466f4f...1b45131. Read the comment docs.

@htynkn
Copy link
Member

htynkn commented Nov 10, 2020

Changes looks good to me.

but there are few things need to be discuss before merge it:

  • are we going to disable travis-ci (it doesn't make sense to have both them, it will waste resource and time)
  • what platform do we use? I prefer linux-jdk8 and linux-jdk11. maybe windows too. but for macos, the lack of action runner will lead a long time to wait
  • different concurrency limit between github action and travis ci (refer to https://www.mail-archive.com/builds@apache.org/msg07478.html)

@AlbumenJ
Copy link
Member Author

hi, @htynkn

what platform do we use? I prefer linux-jdk8 and linux-jdk11. maybe windows too. but for macos, the lack of action runner will lead a long time to wait

yeap, I agree with you. I will disable macOS test case later on.

different concurrency limit between github action and travis ci (refer to https://www.mail-archive.com/builds@apache.org/msg07478.html)

According to Github Community, we can use at least up to 20 workflows concurrently for dubbo repo itself. For now it is enough for our building rate on travis.

@codecov-io
Copy link

codecov-io commented Nov 15, 2020

Codecov Report

Merging #6726 (6337c5c) into master (a2a14dc) will increase coverage by 0.23%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #6726      +/-   ##
============================================
+ Coverage     59.09%   59.32%   +0.23%     
+ Complexity      509      508       -1     
============================================
  Files          1028     1028              
  Lines         41519    41519              
  Branches       6037     6021      -16     
============================================
+ Hits          24536    24633      +97     
+ Misses        14211    14136      -75     
+ Partials       2772     2750      -22     
Impacted Files Coverage Δ Complexity Δ
...he/dubbo/remoting/transport/netty/NettyServer.java 70.17% <0.00%> (-3.51%) 8.00% <0.00%> (-1.00%)
...c/main/java/org/apache/dubbo/rpc/RpcException.java 80.00% <0.00%> (-3.34%) 0.00% <0.00%> (ø%)
...bo/rpc/cluster/support/FailbackClusterInvoker.java 67.21% <0.00%> (-3.28%) 0.00% <0.00%> (ø%)
...in/java/org/apache/dubbo/config/ServiceConfig.java 62.80% <0.00%> (-3.05%) 0.00% <0.00%> (ø%)
.../org/apache/dubbo/qos/legacy/LogTelnetHandler.java 30.30% <0.00%> (-3.04%) 0.00% <0.00%> (ø%)
...va/org/apache/dubbo/remoting/exchange/Request.java 84.09% <0.00%> (-2.28%) 0.00% <0.00%> (ø%)
...apache/dubbo/metadata/rest/RestMethodMetadata.java 50.00% <0.00%> (-1.48%) 0.00% <0.00%> (ø%)
...ache/dubbo/common/compiler/support/ClassUtils.java 49.21% <0.00%> (-1.05%) 0.00% <0.00%> (ø%)
...g/apache/dubbo/registry/consul/ConsulRegistry.java 60.00% <0.00%> (-0.59%) 31.00% <0.00%> (ø%)
...figcenter/file/FileSystemDynamicConfiguration.java 62.38% <0.00%> (-0.48%) 0.00% <0.00%> (ø%)
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2a14dc...6337c5c. Read the comment docs.

@AlbumenJ
Copy link
Member Author

DirtiesContext has been added to all of the test classes in dubbo-config-spring module. In different platform, the exection order of test cases is different and this may causes environment pollution. DirtiesContext can fix it.

Copy link
Member

@htynkn htynkn left a comment

Choose a reason for hiding this comment

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

thanks for your investigate and contribution.

Will merge this change to master soon.
At this stage, I can see some duplicate code in github action yml file, maybe we can use some feature like matrix to make it more clean. those stuff can be done in the future.

@htynkn htynkn merged commit 3fc2d10 into apache:master Nov 16, 2020
@AlbumenJ AlbumenJ deleted the feat/ci branch November 19, 2020 11:57
chickenlj pushed a commit to chickenlj/incubator-dubbo that referenced this pull request Nov 25, 2020
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.

4 participants