-
Notifications
You must be signed in to change notification settings - Fork 47
Extract utbot-spring-framework
module from utbot-framework
#2570
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
Conversation
ef70874
to
ec3beff
Compare
…ecific Remove `CgContext.relevantFieldManagers` field
… to `utbot-spring-test`
Remove duplicated `include("utbot-spring-framework")` in `settings.gradle.kts`
…ementing `CodeGenerationSettingItem` as it's not a setting
ec3beff
to
4b95114
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some unobvious moments
watchdog.measureTimeForActiveCall(perform, "Performing dynamic task") { params -> | ||
val task = kryoHelper.readObject<EngineProcessTask<Any?>>(params.engineProcessTask) | ||
val result = task.perform(kryoHelper) | ||
kryoHelper.writeObject(result) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have only one post-processing task? Perhaps it could be a list of such tasks performed consequently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It's not a post-processing task, the order in witch callbacks are registered in the engine process has nothing to do with the order in which they are called by the IDE process. For example, callback for
findMethodsInClassMatchingSelected
is registered after the callback forgenerate
, despitegenerate
being called afterfindMethodsInClassMatchingSelected
. - If we need to perform multiple tasks we can call
perform
multiple times, the same waygenerate
is called multiple times when we generate tests for multiple classes.
@@ -0,0 +1,39 @@ | |||
val kotlinLoggingVersion: String by rootProject | |||
val rdVersion: String by rootProject | |||
val sootVersion: String by rootProject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need Soot for this module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now NonNullSpeculator
accepts SootField
.
I've tried making engine convert SootField
to FieldId
before passing it to speculativelyCannotProduceNullPointerException
, but it was failing to convert declaring SootClass
to a valid ClassId
for some lambdas, causing ClassNotFoundException
when the default implementation of the speculativelyCannotProduceNullPointerException
tried to retrieve the packageName
which in turned used jClass
.
@@ -0,0 +1,27 @@ | |||
package org.utbot.framework.codegen.domain | |||
|
|||
abstract class SpringModule( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not make it an enum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made it a enum
.
import org.utbot.rd.use | ||
import org.utbot.spring.process.SpringAnalyzerProcess | ||
|
||
class SpringAnalyzerTask( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it true that it does not matter will this task be performed before the symbolic analysis or after? So, can we always consider it as a post-processing task?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's always performed before symbolic analysis and its results are later used to guide type replacement during symbolic execution.
object RelevantFieldManagersProperty : CgContextProperty<MutableList<CgAbstractClassFieldManager>> | ||
|
||
/** | ||
* Managers to process annotated fields of the class under test | ||
* relevant for the current generation type. | ||
*/ | ||
val CgContextOwner.relevantFieldManagers: MutableList<CgAbstractClassFieldManager> | ||
get() = properties.getOrPut(RelevantFieldManagersProperty) { mutableListOf() } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not very obvious why this object and this extension method (which seem to be common for any possible extension to the CgContext
- Spring, or any other in the future) are located in this Spring-specific module
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, there's nothing Spring-specific in declaring class fields. However, right now adding FieldManagers
won't have any effect unless CgSpringVariableConstructor
is used. I've been always in favor of separating Spring-specific and field-declaration-specific logic into 2 decorators of VariableConstructor
instead of one inheritor made an issue regarding that, see #2579.
val springAnalyzerResult = structdef { | ||
field("beanDefinitions", array(beanDefinitionData)) | ||
val performParams = structdef { | ||
field("engineProcessTask", array(PredefinedType.byte)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on its usage, this task seems to always be post-processing. Am I right? If so, please rename it correspondingly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, see reply to the first comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
All spring-specific logic is extracted from
utbot-framework
module toutbot-spring-framework
module.How to test
Regression tests, this PR should not change UtBot semantics at all.
Self-check list