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.

Sign up for GitHub

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

Jump to bottom

Moving ClassHelperTests into Kotlin #2716

Merged
merged 1 commit into from
Jan 18, 2022
Merged

Moving ClassHelperTests into Kotlin #2716

merged 1 commit into from
Jan 18, 2022

Conversation

krmahadevan
Copy link
Member

Fixes # .

Did you remember to?

  • Add test case(s)
  • Update CHANGES.txt
  • Auto applied styling via ./gradlew autostyleApply

We encourage pull requests that:

  • Add new features to TestNG (or)
  • Fix bugs in TestNG

If your pull request involves fixing SonarQube issues then we would suggest that you please discuss this with the
TestNG-dev before you spend time working on it.

Note: For more information on contribution guidelines please make sure you refer our Contributing section for detailed set of steps.

juherr
juherr requested changes Jan 17, 2022
View reviewed changes
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
...ng-core/src/test/kotlin/org/testng/internal/samples/classhelper/issue1339/BabyPandaSample.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/reporters/jq/TimesPanelTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/util/StringsTest.kt Show resolved Hide resolved
@krmahadevan krmahadevan force-pushed the classhelpertests_to_kotlin branch 3 times, most recently from 3216f11 to b47bf57 Compare January 18, 2022 07:16
juherr
juherr reviewed Jan 18, 2022
View reviewed changes
Copy link
Member

@juherr juherr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change is nicer and nicer! 👍

testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
...ng-core/src/test/kotlin/org/testng/internal/samples/classhelper/issue1339/BabyPandaSample.kt Outdated Show resolved Hide resolved
...-core/src/test/kotlin/org/testng/internal/samples/classhelper/issue1339/GrandpaBearSample.kt
@@ -0,0 +1,9 @@
package org.testng.internal.samples.classhelper.issue1339;

open class GrandpaBearSample {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it working if sample classes are not open?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didnt understand this comment. The class cannot be extended if its not open and there are other samples which test the regular use case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You right "not" open means "final".
What about sealed?

The question behind: what is the best keyword?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juherr its still not clear what the question is. I have just ported the classes as they were from Java to Kotlin.

I don't quite understand what the expectations are here.

If these tests were sufficient in Java, are you saying that they arent when it comes to Kotlin ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sealed doesn't exist in java (not yet)
maybe sealed is better than public

it's a global kotlin question because we don't care for tests.

...ng-core/src/test/kotlin/org/testng/internal/samples/classhelper/issue1456/TestClassSample.kt Outdated Show resolved Hide resolved
...e/src/test/kotlin/org/testng/internal/samples/classhelper/misamples/SuperHeroCapabilities.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/util/StringsTest.kt Show resolved Hide resolved
@krmahadevan krmahadevan force-pushed the classhelpertests_to_kotlin branch 2 times, most recently from b9cda1d to fc47f9e Compare January 18, 2022 13:54
@krmahadevan
Copy link
Member Author

@juherr - Fixed all the comments. Please take a look

juherr
juherr approved these changes Jan 18, 2022
View reviewed changes
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt Outdated Show resolved Hide resolved
testng-core/src/test/kotlin/org/testng/internal/ClassHelperTest.kt
return this
}

private fun newXmlClasses(vararg classes: KClass<*>): List<XmlClass> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if it is possible with vararg but it could be awesome to use it like: classes.asXmlClasses()

testng-core/src/test/kotlin/org/testng/reporters/jq/TimesPanelTest.kt Outdated Show resolved Hide resolved
@krmahadevan krmahadevan merged commit fcba70f into testng-team:master Jan 18, 2022
@krmahadevan krmahadevan deleted the classhelpertests_to_kotlin branch January 18, 2022 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juherr juherr juherr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@krmahadevan @juherr