Skip to content

Add GitHub Action to perform regression testing with Apache Calcite bytecode#104

Merged
n1hility merged 3 commits intosmallrye:masterfrom
vlsi:calcite_regression
Jan 22, 2021
Merged

Add GitHub Action to perform regression testing with Apache Calcite bytecode#104
n1hility merged 3 commits intosmallrye:masterfrom
vlsi:calcite_regression

Conversation

@vlsi
Copy link
Contributor

@vlsi vlsi commented Jan 16, 2021

No description provided.

@vlsi vlsi force-pushed the calcite_regression branch 5 times, most recently from bafc6d4 to 4d1b29f Compare January 16, 2021 20:22
@vlsi
Copy link
Contributor Author

vlsi commented Jan 16, 2021

@n1hility , I think the PR is ready. Well, regression jobs fail, however, they highlight the issues.

@vlsi vlsi force-pushed the calcite_regression branch from 4d1b29f to f0dbb56 Compare January 16, 2021 22:28
@n1hility n1hility force-pushed the calcite_regression branch 9 times, most recently from 6e28e4f to 876b787 Compare January 21, 2021 06:04
@n1hility
Copy link
Contributor

n1hility commented Jan 21, 2021

@vlsi I fixed a few more issues that were reported in the logs here, but after fixing the failures persisted in the action, but I was having trouble reproducing this locally (downloading the classes that were post build and then running jandex on them had no issues). Since this error looks like one of the ones that was fixed, I thought I saw a caching issue but �I verified at least part of the build is picking up changes. Not sure if there is anything special about the usage of your gradle plugin but it didn't look like it glancing at the source. Anyway I need to crash but let me know if you spot anything.

@vlsi
Copy link
Contributor Author

vlsi commented Jan 21, 2021

Oh, let me see.

@vlsi
Copy link
Contributor Author

vlsi commented Jan 21, 2021

@n1hility ,

  1. Gradle does not cache the results of failure tasks (it is not implemented in the current Gradle versions)
  2. I can reproduce Required class information is missing failures locally.

For instance, I download calcite-work.tgz, I take calcite/core/build/classes/java/main/org/apache/calcite/adapter/clone/ArrayTable$1.class file (1477 bytes), and I run the following:

public class TestClassBounds {
    @Test
    public void classBounds() throws IOException {
        Indexer indexer = new Indexer();
        ClassInfo info = indexer.index(new FileInputStream("/tmp/ArrayTable$1.class"));
        System.out.println("info = " + info);
        Index index = indexer.complete();
        System.out.println("subclasses");
        index.printSubclasses();
        System.out.println("annotations");
        index.printAnnotations();
    }
}

The error can be reproduced as follows: #92

Are you sure you already fixed the error?

256fae4 yields

java.lang.IllegalStateException: Required class information is missing

	at org.jboss.jandex.Indexer.rebuildNestedType(Indexer.java:989)
	at org.jboss.jandex.Indexer.resolveTypePath(Indexer.java:842)
	at org.jboss.jandex.Indexer.resolveTypeAnnotation(Indexer.java:718)
	at org.jboss.jandex.Indexer.resolveTypeAnnotations(Indexer.java:619)
	at org.jboss.jandex.Indexer.index(Indexer.java:1678)
	at org.jboss.jandex.test.TestClassBounds.classBounds(TestClassBounds.java:15)

@n1hility
Copy link
Contributor

@vlsi thanks for looking and the pointer. For some reason I still can't reproduce on my local setup (mac and linux) [both using the test and the exact class you downloaded] so this is an odd one. I'll dig further!

@n1hility
Copy link
Contributor

Wow it even passes running as GitHub action through the jandex maven run using your exact code and class:
https://github.com/n1hility/jandex/runs/1744092156#step:4:1087

I'll try and kick of the gradle build locally and see if I can get it to fail and debug.

@n1hility
Copy link
Contributor

I finally have a local scenario, looking into it.

@vlsi
Copy link
Contributor Author

vlsi commented Jan 21, 2021

Does that mean the error reproduces in ceratin JVMs only?

@n1hility
Copy link
Contributor

@vlsi yes, so far it looks like it is a compiler bug with anonymous classes that affects some range (don't know yet) of 8 versions. it's a bit of a comedy of errors. In a few cases I had also copied the file over without properly escaping $ So ArrayTable$1.class ended up matching ArrayTable.class, yet when i was disassembling with javap I had escaped it and didn't notice the difference. In other cases I had built with java 11 and it worked.

@n1hility
Copy link
Contributor

oh yeah and then i lost my internet connection. so this bug is resisting being fixed :)

@n1hility
Copy link
Contributor

@vlsi We now have a clean run! 🎉

@n1hility n1hility merged commit 864f11e into smallrye:master Jan 22, 2021
@n1hility
Copy link
Contributor

@vlsi I have released 2.2.3.Final including all of these fixes.

@vlsi
Copy link
Contributor Author

vlsi commented Jan 22, 2021

@n1hility , thanks for the fast release. I'll reiterate with Calcite as the release appears on OSSRH (it is not there yet: https://repo.maven.apache.org/maven2/org/jboss/jandex/ ).

I wonder if you have the changelog somewhere.
It might be helpful to put some notes there, especially taking into account the nature of the issues was moot and the exceptions were quite puzzling.

I guess we'll need to convince @vladmihalcea to bump Hibernate's jandex dependency to prevent the users from running into issues like https://stackoverflow.com/a/65853462/1261287.

@vladmihalcea
Copy link

@vlsi I'm no longer working on the Hibernate project, so you are better off talking to @Sanne about Hibernate issues.

@Sanne
Copy link
Contributor

Sanne commented Jan 24, 2021

Good idea, thanks!

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.

5 participants