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

7903520 - jcov is missing versions of testing dependencies and javatest #40

Merged
merged 1 commit into from Aug 25, 2023

Conversation

judovana
Copy link
Contributor

@judovana judovana commented Aug 22, 2023

jcov now:

  • downloads all necessary dependences, unless present
  • is setting up versions of testng and jcommander
  • is not setting version of javatest, simply gets latest tagged binary
  • gitignores build/template.xml

Progress

  • Change must not contain extraneous whitespace

Issue

  • CODETOOLS-7903520: jcov is missing versions of testing dependencies and javatest (Bug - P2)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jcov.git pull/40/head:pull/40
$ git checkout pull/40

Update a local copy of the PR:
$ git checkout pull/40
$ git pull https://git.openjdk.org/jcov.git pull/40/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 40

View PR using the GUI difftool:
$ git pr show -t 40

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jcov/pull/40.diff

Webrev

Link to Webrev Comment

jcov now:
 - downloads all necessary dependences, unless present
 - is setting up versions of testng and jcommander
 - is not setting version of javatest, simply gets latest tagged binary
 - gitignores build/template.xml
@bridgekeeper
Copy link

bridgekeeper bot commented Aug 22, 2023

👋 Welcome back jvanek! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Aug 22, 2023

@judovana This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

7903520: jcov is missing versions of testing dependencies and javatest

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added ready Ready to be integrated rfr Ready for review labels Aug 22, 2023
@judovana
Copy link
Contributor Author

Note, that I had guessed dependece versions fromcodebase and dates of theirs commits.
And they may not be exactly correct, as the tests are now failing:

...
   [testng] /usr/lib/jvm/java-17-openjdk-17.0.7.0.7-5.fc38.x86_64/bin/java -Djcov.data-saver=com.sun.tdk.jcov.instrument.plugin.FieldsPlugin -cp /home/jvanek/git/ci-jenkins-pipelines/tools/code-tools/jcov/build/plugin_test:/home/jvanek/git/ci-jenkins-pipelines/tools/code-tools/jcov/build/testng-6.9.10.jar:/home/jvanek/git/ci-jenkins-pipelines/tools/code-tools/jcov/JCOV_BUILD/jcov_3.0/jcov.jar:/home/jvanek/git/ci-jenkins-pipelines/tools/code-tools/jcov/JCOV_BUILD/jcov_3.0/jcov_network_saver.jar:/home/jvanek/git/ci-jenkins-pipelines/tools/code-tools/jcov/JCOV_BUILD/test/classes:/home/jvanek/git/ci-jenkins-pipelines/tools/code-tools/jcov/build/jcommander-1.82.jar com.sun.tdk.jcov.instrument.plugin.FieldsClass
   [testng] Comparing [1,2] with [1,2]
   [testng] Comparing [,one,two] with [,one,two]
   [testng] Removing /home/jvanek/git/ci-jenkins-pipelines/tools/code-tools/jcov/build/instr_test
   [testng] Removing /tmp/JDK2364619257894783822
   [testng] Removing /tmp/JDK1946716064096195714
   [testng] Removing user_code
   [testng] PASSED: testInstantiate
   [testng] PASSED: testInstantiateAll
   [testng] PASSED: instrument
   [testng] PASSED: testNested
   [testng] PASSED: testNormal
   [testng] PASSED: testNotRead([Ljava.lang.String;@14a2f921)
   [testng] PASSED: testNotRead([Ljava.lang.String;@2aece37d)
   [testng] PASSED: testNotRead([Ljava.lang.String;@5762806e)
   [testng] PASSED: testNotRead(null)
   [testng] PASSED: testRead([Ljava.lang.String;@5ef60048, "data0", [[Ljava.lang.String;@1d548a08)
   [testng] PASSED: testRead([Ljava.lang.String;@780cb77, "data1", [[Ljava.lang.String;@691a7f8f)
   [testng] PASSED: testRead([Ljava.lang.String;@161b062a, "data2", [[Ljava.lang.String;@17c1bced)
   [testng] PASSED: testRead([Ljava.lang.String;@4034c28c, "data3", [[Ljava.lang.String;@e50a6f6)
   [testng] PASSED: testReasonUninitiated
   [testng] PASSED: testUninitiated
   [testng] PASSED: load
   [testng] PASSED: load
   [testng] PASSED: testNestHostMembers
   [testng] PASSED: fields
   [testng] PASSED: testSaver
   [testng] PASSED: transform
   [testng] PASSED: transform
   [testng] FAILED: instrument
   [testng] java.lang.AssertionError
   [testng] 	at com.sun.tdk.jcov.instrument.asm.BranchCodeMethodAdapter.computeEndBCIsAndFoldInExits(BranchCodeMethodAdapter.java:450)
   [testng] 	at com.sun.tdk.jcov.instrument.asm.BranchCodeMethodAdapter.visitEnd(BranchCodeMethodAdapter.java:524)
   [testng] 	at org.objectweb.asm.MethodVisitor.visitEnd(MethodVisitor.java:783)
   [testng] 	at org.objectweb.asm.MethodVisitor.visitEnd(MethodVisitor.java:783)
   [testng] 	at org.objectweb.asm.ClassReader.readMethod(ClassReader.java:1495)
   [testng] 	at org.objectweb.asm.ClassReader.accept(ClassReader.java:721)
   [testng] 	at com.sun.tdk.jcov.instrument.asm.ClassMorph.morph(ClassMorph.java:275)
   [testng] 	at com.sun.tdk.jcov.Instr$1.instrument(Instr.java:303)
   [testng] 	at com.sun.tdk.jcov.insert.AbstractUniversalInstrumenter.processClassFile(AbstractUniversalInstrumenter.java:186)
   [testng] 	at com.sun.tdk.jcov.insert.AbstractUniversalInstrumenter.instrument(AbstractUniversalInstrumenter.java:620)
   [testng] 	at com.sun.tdk.jcov.insert.AbstractUniversalInstrumenter.instrument(AbstractUniversalInstrumenter.java:546)
   [testng] 	at com.sun.tdk.jcov.Instr.instrumentFiles(Instr.java:241)
   [testng] 	at com.sun.tdk.jcov.Instr.run(Instr.java:576)
   [testng] 	at com.sun.tdk.jcov.tools.JCovCMDTool.run(JCovCMDTool.java:164)
   [testng] 	at com.sun.tdk.jcov.instrument.instr.InstrTest.instrument(Unknown Source)
   [testng] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   [testng] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
   [testng] 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   [testng] 	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
   [testng] 	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:86)
   [testng] 	at org.testng.internal.Invoker.invokeMethod(Invoker.java:643)
   [testng] 	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:820)
   [testng] 	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1128)
   [testng] 	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
   [testng] 	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
   [testng] 	at org.testng.TestRunner.privateRun(TestRunner.java:782)
   [testng] 	at org.testng.TestRunner.run(TestRunner.java:632)
   [testng] 	at org.testng.SuiteRunner.runTest(SuiteRunner.java:366)
   [testng] 	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:361)
   [testng] 	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:319)
   [testng] 	at org.testng.SuiteRunner.run(SuiteRunner.java:268)
   [testng] 	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
   [testng] 	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
   [testng] 	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1244)
   [testng] 	at org.testng.TestNG.runSuitesLocally(TestNG.java:1169)
   [testng] 	at org.testng.TestNG.run(TestNG.java:1064)
   [testng] 	at org.testng.TestNG.privateMain(TestNG.java:1385)
   [testng] 	at org.testng.TestNG.main(TestNG.java:1354)
   [testng] 
   [testng] FAILED: testJREInstr
   [testng] java.util.NoSuchElementException: No value present
   [testng] 	at java.base/java.util.Optional.get(Optional.java:143)
   [testng] 	at com.sun.tdk.jcov.instrument.jreinstr.JREInstrTest.testJREInstr(Unknown Source)
   [testng] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   [testng] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
   [testng] 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   [testng] 	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
   [testng] 	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:86)
   [testng] 	at org.testng.internal.Invoker.invokeMethod(Invoker.java:643)
   [testng] 	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:820)
   [testng] 	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1128)
   [testng] 	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
   [testng] 	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
   [testng] 	at org.testng.TestRunner.privateRun(TestRunner.java:782)
   [testng] 	at org.testng.TestRunner.run(TestRunner.java:632)
   [testng] 	at org.testng.SuiteRunner.runTest(SuiteRunner.java:366)
   [testng] 	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:361)
   [testng] 	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:319)
   [testng] 	at org.testng.SuiteRunner.run(SuiteRunner.java:268)
   [testng] 	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
   [testng] 	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
   [testng] 	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1244)
   [testng] 	at org.testng.TestNG.runSuitesLocally(TestNG.java:1169)
   [testng] 	at org.testng.TestNG.run(TestNG.java:1064)
   [testng] 	at org.testng.TestNG.privateMain(TestNG.java:1385)
   [testng] 	at org.testng.TestNG.main(TestNG.java:1354)
   [testng] 
   [testng] FAILED: testJREInstr
   [testng] java.lang.AssertionError: expected [0] but found [1]
   [testng] 	at org.testng.Assert.fail(Assert.java:94)
   [testng] 	at org.testng.Assert.failNotEquals(Assert.java:513)
   [testng] 	at org.testng.Assert.assertEqualsImpl(Assert.java:135)
   [testng] 	at org.testng.Assert.assertEquals(Assert.java:116)
   [testng] 	at org.testng.Assert.assertEquals(Assert.java:389)
   [testng] 	at org.testng.Assert.assertEquals(Assert.java:399)
   [testng] 	at com.sun.tdk.jcov.instrument.plugin.jreinstr.JREInstrTest.testJREInstr(Unknown Source)
   [testng] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   [testng] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
   [testng] 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   [testng] 	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
   [testng] 	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:86)
   [testng] 	at org.testng.internal.Invoker.invokeMethod(Invoker.java:643)
   [testng] 	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:820)
   [testng] 	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1128)
   [testng] 	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
   [testng] 	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
   [testng] 	at org.testng.TestRunner.privateRun(TestRunner.java:782)
   [testng] 	at org.testng.TestRunner.run(TestRunner.java:632)
   [testng] 	at org.testng.SuiteRunner.runTest(SuiteRunner.java:366)
   [testng] 	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:361)
   [testng] 	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:319)
   [testng] 	at org.testng.SuiteRunner.run(SuiteRunner.java:268)
   [testng] 	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
   [testng] 	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
   [testng] 	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1244)
   [testng] 	at org.testng.TestNG.runSuitesLocally(TestNG.java:1169)
   [testng] 	at org.testng.TestNG.run(TestNG.java:1064)
   [testng] 	at org.testng.TestNG.privateMain(TestNG.java:1385)
   [testng] 	at org.testng.TestNG.main(TestNG.java:1354)
   [testng] 
   [testng] SKIPPED: run
   [testng] java.lang.Throwable: Method InstrTest.run()[pri:0, instance:com.sun.tdk.jcov.instrument.instr.InstrTest@6500df86] depends on not successfully finished methods
   [testng] 	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1037)
   [testng] 	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
   [testng] 	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
 ...
   [testng] 	at org.testng.TestRunner.run(TestRunner.java:632)
   [testng] 	at org.testng.SuiteRunner.runTest(SuiteRunner.java:366)
   [testng] 	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:361)
   [testng] 	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:319)
   [testng] 	at org.testng.SuiteRunner.run(SuiteRunner.java:268)
   [testng] 	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
   [testng] 	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
   [testng] 	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1244)
   [testng] 	at org.testng.TestNG.runSuitesLocally(TestNG.java:1169)
   [testng] 	at org.testng.TestNG.run(TestNG.java:1064)
   [testng] 	at org.testng.TestNG.privateMain(TestNG.java:1385)
   [testng] 	at org.testng.TestNG.main(TestNG.java:1354)
   [testng] 
   [testng] 
   [testng] ===============================================
   [testng]     TestNG tests
   [testng]     Tests run: 29, Failures: 3, Skips: 4
   [testng] ===============================================
   [testng] 
   [testng] 
   [testng] ===============================================
   [testng] jcov
   [testng] Total tests run: 29, Failures: 3, Skips: 4
   [testng] ===============================================
   [testng] 
   [testng] [TestNG] Time taken by org.testng.reporters.JUnitReportReporter@2eafffde: 15 ms
   [testng] [TestNG] Time taken by [FailedReporter passed=0 failed=0 skipped=0]: 8 ms
   [testng] [TestNG] Time taken by org.testng.reporters.XMLReporter@8807e25: 17 ms
   [testng] [TestNG] Time taken by org.testng.reporters.EmailableReporter2@3419866c: 12 ms
   [testng] [TestNG] Time taken by org.testng.reporters.SuiteHTMLReporter@4f47d241: 12 ms
   [testng] [TestNG] Time taken by org.testng.reporters.jq.Main@27ddd392: 25 ms
   [testng] The tests failed.

BUILD SUCCESSFUL
Total time: 5 seconds

@mlbridge
Copy link

mlbridge bot commented Aug 22, 2023

Webrevs

@lkuskov
Copy link
Collaborator

lkuskov commented Aug 23, 2023

LGTM. Since the test target is part of the development stage, it's okay to add a dependency on external trusted resources.

@judovana
Copy link
Contributor Author

Thanx! Are you ok with the versions used. despite the test failures?

@lkuskov
Copy link
Collaborator

lkuskov commented Aug 23, 2023

IMO, in this case, failing tests are the subject to analyze tests, implementation, and fix what is necessary. This change doesn't affect jcov at all.

@judovana
Copy link
Contributor Author

You are most likely right. Still with various versions of testng and of jcommander, it was failing in various ways.

Those failed tests seems to come from 8deab26 , so maybe @shurymury may confirm?

@judovana
Copy link
Contributor Author

Bu if you are ok with this as it is, the versions may fixed later.

@judovana
Copy link
Contributor Author

Note, that I'm not an commiter, feel free to merge on my behalf

@lkuskov lkuskov merged commit ce686b1 into openjdk:master Aug 25, 2023
1 check passed
@lkuskov
Copy link
Collaborator

lkuskov commented Aug 25, 2023

the commit is integrated.

@judovana
Copy link
Contributor Author

tyvm!

judovana added a commit to judovana/ci-jenkins-pipelines that referenced this pull request Aug 28, 2023
…adoptium#795

Adapted to freshly merged openjdk/jcov#40
- latest tag :
  - still wget smtools manually, no change here,
  - wget javatest.jar from adoptium jenkins
  - both will need adjsut once current tip gets tag
- tip no longer pulls any deps
  - they are pulled by jcov ant task (including the adoptium jenkins)
  - tip no longer set proeprty (it is correct from ant tasks)
  - readme is still geenrated as it was, only the asmtools version is
    read from jcov/build/properties
@openjdk openjdk bot mentioned this pull request Aug 29, 2023
1 task
karianna added a commit to adoptium/ci-jenkins-pipelines that referenced this pull request Aug 30, 2023
…#795 (#796)

* jcov failed to build becasue of change of depndencies in its ant build #795

Adapted to freshly merged openjdk/jcov#40
- latest tag :
  - still wget smtools manually, no change here,
  - wget javatest.jar from adoptium jenkins
  - both will need adjsut once current tip gets tag
- tip no longer pulls any deps
  - they are pulled by jcov ant task (including the adoptium jenkins)
  - tip no longer set proeprty (it is correct from ant tasks)
  - readme is still geenrated as it was, only the asmtools version is
    read from jcov/build/properties

* Minor fix in asm-tools and Adoptium words

* Update tools/code-tools/jcov.sh

Co-authored-by: Shelley Lambert <slambert@gmail.com>

---------

Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>
Co-authored-by: Shelley Lambert <slambert@gmail.com>
@shurymury
Copy link
Collaborator

@judovana Thank you for the enhancement! It is way easier to build from scratch now.

I do however have a couple of comments, which I expressed in a bug and an enhancement:
https://bugs.openjdk.org/browse/CODETOOLS-7903550
https://bugs.openjdk.org/browse/CODETOOLS-7903551

Would you be willing to take a look on these two?

@judovana
Copy link
Contributor Author

judovana commented Sep 6, 2023

Yup, thanx for feedback. I had read both bugzillas, and will elaborate as quickly as time allows. Both looks reasonable. (although for CODETOOLS-7903550, I would vote for symlink O:)

luhenry pushed a commit to luhenry/adoptium-ci-jenkins-pipelines that referenced this pull request Feb 3, 2024
…adoptium#795 (adoptium#796)

* jcov failed to build becasue of change of depndencies in its ant build adoptium#795

Adapted to freshly merged openjdk/jcov#40
- latest tag :
  - still wget smtools manually, no change here,
  - wget javatest.jar from adoptium jenkins
  - both will need adjsut once current tip gets tag
- tip no longer pulls any deps
  - they are pulled by jcov ant task (including the adoptium jenkins)
  - tip no longer set proeprty (it is correct from ant tasks)
  - readme is still geenrated as it was, only the asmtools version is
    read from jcov/build/properties

* Minor fix in asm-tools and Adoptium words

* Update tools/code-tools/jcov.sh

Co-authored-by: Shelley Lambert <slambert@gmail.com>

---------

Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>
Co-authored-by: Shelley Lambert <slambert@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Ready to be integrated rfr Ready for review
3 participants