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

Extract CLI logic from TestNG core #2612

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

juherr
Copy link
Member

@juherr juherr commented Jul 16, 2021

Did you remember to?

  • Add test case(s)
  • Update CHANGES.txt

https://github.com/remkop/picocli is the new defacto library for CLI.
Even if there is no plan to change the default implementation, JCommander was deeply linked.

The goal of this PR is to remove the strong dependency to JCommander.

@krmahadevan
Copy link
Member

@juherr - Please give me some more time to go through this implementation (maybe this weekend). I liked this decoupling 👍
I will try to wrap up whatever observations I had by that time.

One thing. Do you think we can start preferring to use Streams and lambdas over regular for loops ?

@krmahadevan
Copy link
Member

@juherr - I tried checking out your branch and then running the tests via ./gradlew clean test. I see a lot of test failures.
I also tried running a sample test case from within IntelliJ, I am seeing

Exception in thread "main" java.lang.InstantiationError: org.testng.CommandLineArgs
	at com.intellij.rt.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:106)

Process finished with exit code 1

Please let me know if I am doing anything incorrectly here.

@juherr juherr marked this pull request as draft July 18, 2021 11:05
@juherr
Copy link
Member Author

juherr commented Jul 18, 2021

@krmahadevan Thanks for the feedback. I moved it to draft because I didn't check tests yet.

If the move looks good enough for you, I will finish it asap.

@krmahadevan
Copy link
Member

@juherr - Sure. I am fine with this change. Please help take it forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants