search

pivotal-cf / java-cfenv

Apache License 2.0
91 stars 56 forks source link

Arrange assertion #264

Open anthonydahanne opened 1 month ago

anthonydahanne commented 1 month ago

With 

./gradlew clean :java-cfenv-test-support:generatePomFileForMavenJavaPublication

Before (assert 2):

<dependencies>
        <dependency>
            <groupId>io.pivotal.cfenv</groupId>
            <artifactId>java-cfenv</artifactId>
            <version>3.1.6-SNAPSHOT</version>
            <scope>compile</scope>
        </dependency>
        <dependency>
            <groupId>io.pivotal.cfenv</groupId>
            <artifactId>java-cfenv</artifactId>
            <version>3.1.6-SNAPSHOT</version>
            <scope>compile</scope>
            <type>test-jar</type>
        </dependency>
[...]
</dependencies>

after (assert 1):

<dependencies>
    <dependency>
      <groupId>io.pivotal.cfenv</groupId>
      <artifactId>java-cfenv</artifactId>
      <version>3.1.6-SNAPSHOT</version>
      <scope>compile</scope>
      <type>test-jar</type>
    </dependency>
</dependencies>

It's as if Gradle "optimized" the 2 deps into 1 - for most probably (I'm gonna find out) the same classpath result in Maven

anthonydahanne commented 1 month ago

Gradle team has confirmed they now de duplicate dependencies; hence only 1 now.

To revert to the previous behaviour, it was suggested to:

More specifically, java-cfenv-test-support defines the following two dependencies:

api project(':java-cfenv')
api testFixtures(project(':java-cfenv'))

Since test fixtures are not compatible with maven, Gradle emits the same entry in the POM for each of these dependencies — these entries are now deduplicated (before withXml is executed). We do warn that the metadata being published is incompatible with Maven, however, those warnings are being suppressed here. I would recommend either Adding the entry manually instead of relying on Gradle to add it (More preferably) Move the contents of java-cfenv’s test fixtures to java-cfenv-test-support if you want to remain Maven compatible. A better story for this issue would be solved by this issue, where we would do a little better with the maven compatibility by emitting a dependency with a classifier for the testFixtures dependency instead of merging it with the normal project dependency.

Since it's not convenient at that time to change the project layout (like having a separate project only contain the testFixtures), for now this PR changes the assertion to 1 dependency, and then re adding the dep with a different type.

I tried 

                    if (project.name == 'java-cfenv-test-support') {
                        def cfenvDependencies = pomNode.get('dependencies')[0].findAll { it.get('artifactId')[0].text() == 'java-cfenv' }
                        def clone = cfenvDependencies[0].clone()
                        assert cfenvDependencies.size() == 1
                        // see https://github.com/pivotal-cf/java-cfenv/pull/264#issuecomment-2105127180
                        cfenvDependencies[0].appendNode('type', "test-jar")
                        cfenvDependencies.add(clone)
                        assert cfenvDependencies.size() == 2
                    }

but it still got deduplicated.

Let's try tpopublish with just this one dependency; I can't think that's gonna cause issues downstream