Skip to content

Fixed the EnvUtilTest to be compatible with Windows OS and OpenJDK#2570

Closed
s4ravanan wants to merge 8 commits intoapache:masterfrom
s4ravanan:apm-data-unit-test
Closed

Fixed the EnvUtilTest to be compatible with Windows OS and OpenJDK#2570
s4ravanan wants to merge 8 commits intoapache:masterfrom
s4ravanan:apm-data-unit-test

Conversation

@s4ravanan
Copy link
Copy Markdown

Please answer these questions before submitting your issue.

  • Why do you submit this issue?
  • Bug
    The Unit test EnvUtilTest.java case in apm-datacarrier module is not passing in Windows OS 10.

Question

  • What do you want to know?
    Can we build the code in Windows Machine?

Bug

  • Which version of SkyWalking, OS and JRE?
    Skywalking - Latest Code base as of Master
    OS - Windows 10
    JRE - tried with Open JDK from Redhat and from Oracle JDK 8
    Open JDK:
    openjdk version "1.8.0_201-2-redhat"
    OpenJDK Runtime Environment (build 1.8.0_201-2-redhat-b09)

Oracle JDK
java version "1.8.0_211"
Java(TM) SE Runtime Environment (build 1.8.0_211-b12)

  • Which company or project?
    I am trying the build source code in local system to write more unit tests, however the build is failing before that.
  • What happen?
    when I try to build apm-datacarrier the build fails with the following error

[INFO] Running org.apache.skywalking.apm.commons.datacarrier.EnvUtilTest
[ERROR] Tests run: 2, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.013 s <<< FAILURE! - in org.apache.skywalking.apm.commons.datacarrier.EnvUtilTest
[ERROR] getInt(org.apache.skywalking.apm.commons.datacarrier.EnvUtilTest) Time elapsed: 0.002 s <<< FAILURE!
java.lang.AssertionError: expected:<123> but was:<234>
at org.apache.skywalking.apm.commons.datacarrier.EnvUtilTest.getInt(EnvUtilTest.java:56)

[ERROR] getLong(org.apache.skywalking.apm.commons.datacarrier.EnvUtilTest) Time elapsed: 0 s <<< FAILURE!
java.lang.AssertionError: expected:<12345678901234567> but was:<123>
at org.apache.skywalking.apm.commons.datacarrier.EnvUtilTest.getLong(EnvUtilTest.java:62)

[INFO] Running org.apache.skywalking.apm.commons.datacarrier.partition.ProducerThreadPartitionerTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.013 s - in org.apache.skywalking.apm.commons.datacarrier.partition.ProducerThreadPartitionerTest
[INFO] Running org.apache.skywalking.apm.commons.datacarrier.partition.SimpleRollingPartitionerTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.013 s - in org.apache.skywalking.apm.commons.datacarrier.partition.SimpleRollingPartitionerTest
[INFO]
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] EnvUtilTest.getInt:56 expected:<123> but was:<234>
[ERROR] EnvUtilTest.getLong:62 expected:<12345678901234567> but was:<123>
[INFO]
[ERROR] Tests run: 17, Failures: 2, Errors: 0, Skipped: 0


Requirement or improvement

After little bit of search I found that the code doesn't work on Windows to make it work we need to follow the steps as mentioned in link:
https://stackoverflow.com/a/38073822

I have created the pull request with the necessary change.
Kindly review the same and let me know the results.

Thanks and Regards,
Saravanan Kandaswamy

Copy link
Copy Markdown
Member

@wu-sheng wu-sheng 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 if we are trying to make sure multiple JDKs/OSs compatible, we need to follow these two

  1. https://docs.travis-ci.com/user/languages/java/#testing-against-multiple-jdks
  2. https://docs.travis-ci.com/user/multi-os/

I am not sure the travis could provide the CI fast enough, but worth to try.

@wu-sheng wu-sheng added this to the 6.2.0 milestone May 1, 2019
@wu-sheng wu-sheng added agent Language agent related. test Test requirements about performance, feature or before release. labels May 1, 2019
@coveralls
Copy link
Copy Markdown

coveralls commented May 1, 2019

Coverage Status

Coverage remained the same at 16.416% when pulling 702296a on saravanan-kandaswamy:apm-data-unit-test into 4e71085 on apache:master.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented May 1, 2019

Could you try to add more jdk and OS versions to travis yml?

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented May 1, 2019

Maybe I need to set other CI platform to support windows compile. @liuhaoyang

@wu-sheng wu-sheng requested a review from liuhaoyang May 1, 2019 21:10
@wu-sheng wu-sheng removed this from the 6.2.0 milestone May 2, 2019
@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented May 2, 2019

@saravanan-kandaswamy Please help us verify this PR. I don't have windows and many jdk versions at local. Need you work on CI to support those and avoid this in the future.

@s4ravanan
Copy link
Copy Markdown
Author

@saravanan-kandaswamy Please help us verify this PR. I don't have windows and many jdk versions at local. Need you work on CI to support those and avoid this in the future.

I have no knowledge on Travis-CI, i do know Jenkins though, let me know what kind of activity i need to do to setup the CI for windows.

@liuhaoyang
Copy link
Copy Markdown
Member

Maybe I need to set other CI platform to support windows compile. @liuhaoyang

we use the appveyor platform to support windows compile. We have previously created an appveyor account for skyapm-dotnet and I will use it for testing.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented May 5, 2019

@liuhaoyang https://issues.apache.org/jira/browse/INFRA-18330 JIRA ticket submitted. Asking Apache INFRA to open it.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented May 5, 2019

OpenJDK passed in CI #2583

@wu-sheng wu-sheng added the wontfix This will not be worked on label May 7, 2019
@wu-sheng wu-sheng added this to the 6.2.0 milestone May 7, 2019
@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented May 7, 2019

Fixed in #2593

@wu-sheng wu-sheng closed this May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Language agent related. test Test requirements about performance, feature or before release. wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants