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

Possible to exclude the testng-assert module when using testng #3197

Open
mpet opened this issue Dec 13, 2024 · 24 comments
Open

Possible to exclude the testng-assert module when using testng #3197

mpet opened this issue Dec 13, 2024 · 24 comments

Comments

@mpet
Copy link

mpet commented Dec 13, 2024

TestNG Version

v7.10.2

Suggestion

Hi
In our team we discussed this:
Our majority of TestNG testcases uses https://joel-costigliola.github.io/assertj/ for assertions.
But when users write TestNG unit tests the first alternative that pops up is org.testng.Assert.
Would it be possible to make this module optional? This is especially important for users that
use TestNG for executing tests and assertj to assert.

br,

//mikael

@juherr
Copy link
Member

juherr commented Dec 13, 2024

Currently, TestNG is released into a single artifact, but the project is already modularized and assertions are in their own module.
Maybe we should consider releasing TestNG artifact without assertions plus an extra artifact for assertions.

@krmahadevan WDYT

Related to #2649

@krmahadevan
Copy link
Member

@juherr - That would mean that we would need to do 3 artifact releases

  • Assertions library from TestNG
  • TestNG without assertions
  • TestNG with assertions (it would consume the assertion library)

Is this what we are looking at ?

@juherr
Copy link
Member

juherr commented Dec 13, 2024

Only assertions and testng without assertions.

It means the upgrade will need to add assertions as dependencies if used.

@krmahadevan
Copy link
Member

Wouldn't that cause backward compatibility issues for users? Yes, we can send out announcements etc., but just calling out.

Should we do this as part of a minor release or a major release?

@martinaldrin
Copy link

I agree, release the assert library as a separate artifact.
Mark it as optional dependency in the pom.xml that user can add if they need it.

@juherr
Copy link
Member

juherr commented Dec 13, 2024

@krmahadevan yes, it will make a little regression that must be documented

@mpet
Copy link
Author

mpet commented Dec 20, 2024

@juherr is it accepted?

@juherr
Copy link
Member

juherr commented Dec 20, 2024

@mpet I think it will accepted when someone will propose a pull request for that.
😉

@mpet
Copy link
Author

mpet commented Dec 20, 2024

@juherr I guess we need to exclude the asserts from shaded jar too. Maybe it should not be included at all. What do you say?

@juherr
Copy link
Member

juherr commented Dec 20, 2024

@mpet excluded from the shaded jar and deployed as a standalone artifact.

@mpet
Copy link
Author

mpet commented Dec 20, 2024

ok. @juherr Any input on what should be in the build file for testng-asserts. I guess some of the stuff in
https://github.com/testng-team/testng/blob/master/build.gradle.kts should be reused.

@mpet
Copy link
Author

mpet commented Dec 20, 2024

I cannot see where the 'publish' is done

So I would need your assistance of what parts to include the ..\git\testng\testng-asserts\testng-asserts-build.gradle.kts
since it is not a regular release.

I have added this, so far:

plugins {
    id("testng.java-library")
    id("maven-publish")
}

group = "org.testng" 
version = "7.8.0"
description = "TestNG Assertions Library"

dependencies {
    implementation(projects.testngCollections) {
        because("Lists.newArrayList")
    }

    testImplementation("org.testng:testng:7.3.0") {
        because("we need testng to test assertions")
    }
    testImplementation(projects.testngTestKit)
}



publishing {
    publications {
        create<MavenPublication>("mavenJava") {
            from(components["java"])
            pom {
                name = "TestNG Assertions"
                description = "Assertions library for TestNG"
                url = "https://testng.org"
                licenses {
                    license {
                        name = "Apache License, Version 2.0"
                        url = "http://www.apache.org/licenses/LICENSE-2.0.txt"
                    }
                }
                developers {
                    developer {
                        id = "testng-developers"
                        name = "TestNG Developers"
                        email = "[email protected]"
                    }
                }
                scm {
                    connection = "scm:git:git://github.com/testng-team/testng.git"
                    developerConnection = "scm:git:ssh://[email protected]/testng-team/testng.git"
                    url = "https://github.com/testng-team/testng"
                }
            }
        }
    }

What am I missing?

How can I test a release and make sure it has the correct info?

@mpet
Copy link
Author

mpet commented Dec 20, 2024

@krmahadevan do you know the release process and which parts that I need?

@krmahadevan
Copy link
Member

@mpet - The release process is documented here: https://github.com/testng-team/testng/wiki/TestNG-release-process

I believe that we would perhaps need to create a github actions workflow that would release assert library.

@vlsi - It would be good if you could help share some guidance around here.

@mpet
Copy link
Author

mpet commented Dec 20, 2024

yes it seems to have a dedicated flow where you set the RC to be released. Not sure what is need in the build to support this. But I hope @vlsi can shed some light in this matter...

@mpet
Copy link
Author

mpet commented Dec 23, 2024

@krmahadevan is @vlsi still active? How can we make this go forward...

@krmahadevan
Copy link
Member

@mpet - Vlad helped setup the build system for TestNG. He can help more aptly. Given that this is a holiday season I would expect latency in people responding.

I am not well versed with Gradle as a build system so I don't have an answer and will need to spend time trying to figure out.

So we wait till @vlsi gets back or you can try taking a jab at this.

@vlsi
Copy link
Contributor

vlsi commented Dec 23, 2024

The key question is the way to handle backward compatibility.

We could move asserts to a different artifact, and use it as a regular dependency.
Then, at some later version (major one?), the dependency could be removed, so the ones who need it should add it manually.

The sad thing is that ideally, different jars should not have the same packages.
Currently, Assert lives in org.testng which is the same package that keeps many other useful interfaces like ITestContext and so on.

Unfortunately, the migration from org.testng.Assert to something like org.testng.assertions.Assert would be a bit more complicated (and it would probably require adding an artifact with a different name.


In other words:

Current state:

TestNG 7.10.2
testng.jar "everything in a single jar"

TestNG 8.0.0
testng.jar "except assertions"
testng-assertions.jar "org.testng.Assert"

TestNG 9.0.0
testng.jar "except assertions"
testng-assertions.jar "org.testng.Assert as deprecated". There could be automatic migrations like OpenRewrite rewrite-testing-frameworks or InlineMe
testng-assertions2.jar "org.testng.assertions.Assert"

TestNG 15.0.0
testng.jar "except assertions"
drop testng-assertions.jar
testng-assertions2.jar "org.testng.assertions.Assert"


WDYT?

@juherr
Copy link
Member

juherr commented Dec 23, 2024

Thanks for the feedback.
I think it could go faster

What if "org.testng.assertions.Assert" is introduced and "org.testng.Assert" is deprecated in the next minor release?

What if "org.testng.assertions" is moved into its dedicated artifact, and core depends on it in the next minor release?

Both moves should not break anything, right?

@vlsi
Copy link
Contributor

vlsi commented Dec 23, 2024

What if "org.testng.assertions.Assert" is introduced and "org.testng.Assert" is deprecated in the next minor release?

Sure it is possible to combine the changes. I just assumed it would take some time to add org.testng.assertions, etc, etc.


By the way, my plan missed a release that would exclude "testng-assertions" from the default dependency of testng.jar.
Frankly, I am not sure if moving org.testng.Assert to its own jar is backward-compatible as the users might have used something like -classpath testng.jar

@juherr
Copy link
Member

juherr commented Dec 24, 2024

Right. It can break with cli runner.

Then I propose to create the new artifact with the new package, and deprecate assertions from the base package in a next minor version.

And remove deprecated in the next major as usual.

Wdyt @krmahadevan ?

@krmahadevan
Copy link
Member

@juherr

So in TestNG 7.11.0 -

  • We deprecate org.testng.Assert
  • We create a new gradle module named testng-asserts
  • introduce org.testng.assertions.Assert in testng-asserts
  • publish testng-asserts as a separate artifact into Maven Central
  • Make this announcement as part of the release notes and release publishing activites.

In TestNG 7.13.0 we remove org.testng.Assert from the testng uber jar and be done with this activity.

Is this a fair understanding of the expectations here?

@vlsi
Copy link
Contributor

vlsi commented Dec 24, 2024

In TestNG 7.13.0 we remove org.testng.Assert from the testng uber jar and be done with this activity

I would ask you not doing so.
It takes time for the consumers to adapt, so "removal" should better be delayed by 5 or so years.

@krmahadevan
Copy link
Member

@vlsi Personally I wouldn't bother removing Asserts at all. A test runner is supposed to provide the basic assertion capabilities. If someone doesn't want to use them then they can always use something else. It's just a matter of preference.

But the ask in this issue itself was that, they felt it was not an optimal experience. If we are going to keep the assertion classes for that long then we might as well not remove them 🫣

I don't have a strong opinion on this either way because there are straightforward ways of avoiding it, ranging from code reviews that enforce this avoidance to using the enforcer plug-in to ensure that the testng powered asserts are not used in the code base.

I was merely trying to figure out the list of activities that need to be done ( along with some timelines ) to accomplish the ask in this issue.

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

No branches or pull requests

5 participants