Thanks, looks good to me!
> On Mar 19, 2024, at 10:19 AM, Abe Ratnofsky <a...@aber.io> wrote:
>
> Here's where ClassLoader names were introduced in JDK9:
> https://bugs.java.com/bugdatabase/view_bug?bug_id=6479237
>
> And where Jacoco introduced support for the exclclassloader agent option:
> https://github.com/jacoco/jacoco/commit/b8ee4efe9c2ba93485fe5d9e25340113efc2390b
>
> My understanding is that names only exist to improve error messages
> (ClassCastException specifically), and other diagnostic features like
> Jacoco's exclusions. I am not aware of any behavior-impacting reasons to name
> a ClassLoader, or anything this would break by adding ClassLoader names, but
> I'm testing my changes now and will report back. Naming is done just by
> overriding ClassLoader.getName:
> https://docs.oracle.com/javase/9/docs/api/java/lang/ClassLoader.html#getName--
>
>> On Mar 18, 2024, at 3:48 PM, David Capwell <dcapw...@apple.com> wrote:
>>
>> Are there any side effects to naming ClassLoaders? How do we do this?
>>
>> I am +1 to the idea but don’t know enough to know what negatives could
>> happen.
>>
>>> On Mar 17, 2024, at 7:25 AM, Josh McKenzie <jmcken...@apache.org> wrote:
>>>
>>> +1 from me
>>>
>>>> If classloaders are appropriately named in the current versions of
>>>> Cassandra, we should be able to test upgrade paths to that version without
>>>> updating the older branches or building new jvm-dtest JARs for them.
>>> Pretty sure Caleb was wrestling w/something recently that might have
>>> benefited from being able to differentiate which ClassLoader was sad; in
>>> general this seems like it'd be a big help to debugging startup / env
>>> issues, assuming it doesn't break anything. :)
>>>
>>> On Fri, Mar 15, 2024, at 4:41 PM, Abe Ratnofsky wrote:
>>>> Hey folks,
>>>>
>>>> I'm working on gathering coverage data across all of our test suites. The
>>>> jvm-dtest-upgrade suite is currently unfriendly to Jacoco: it uses classes
>>>> from multiple versions of Cassandra but with the same class names, so
>>>> Jacoco's analysis fails due to "Can't add different class with same
>>>> name"[1]. We need a way to exclude alternate-version classes from Jacoco's
>>>> analysis, so we can get coverage for the current version of Cassandra.
>>>>
>>>> Jacoco supports exclusion of classes based on class name or classloader
>>>> name, but the class names are frequently usually identical across
>>>> Cassandra versions. The jvm-dtest framework doesn't name classloaders, so
>>>> we can't use that either.
>>>>
>>>> I'd like to propose that we name the jvm-dtest InstanceClassLoader
>>>> instances so that some can be excluded from Jacoco's analysis. Instances
>>>> that create new InstanceClassLoaders should be able to provide an
>>>> immutable name in the constructor. InstanceClassLoader names should
>>>> indicate whether they're for the current version of Cassandra (where
>>>> coverage should be collected) or an alternate version. If classloaders are
>>>> appropriately named in the current versions of Cassandra, we should be
>>>> able to test upgrade paths to that version without updating the older
>>>> branches or building new jvm-dtest JARs for them.
>>>>
>>>> Any objections or alternate approaches?
>>>>
>>>> --
>>>> Abe
>>>>
>>>> [1]: More specifically: Jacoco uses class IDs to map the analysis data
>>>> that's produced during text execution to .class files. I'm configuring the
>>>> Jacoco agent's classdumpdir to ensure that the classes loaded during
>>>> execution are the same classes that are analyzed during report generation,
>>>> as is recommended. When we build the alternate version JARs for
>>>> jvm-dtest-upgrade, we end up with multiple classes with the same name but
>>>> different IDs.
>>
>
