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

Other class is not mocked as required #747 #1033

Merged
merged 3 commits into from
Oct 11, 2022

Conversation

Markoutte
Copy link
Collaborator

@Markoutte Markoutte commented Sep 29, 2022

Description

Adds mocks for values that are passed into a method by fuzzer. Note, that fuzzer cannot mock any internal values if they are met in the code block.

Fixes #747

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Automated Testing

org.utbot.framework.plugin.api.MockOfObjectModelProviderTest

Manual Scenario

Reproduce examples from the issue.

Checklist:

  • The change followed the style guidelines of the UTBot project
  • Self-review of the code is passed
  • The change contains enough commentaries, particularly in hard-to-understand areas
  • New documentation is provided or existed one is altered
  • No new warnings
  • New tests have been added
  • All tests pass locally with my changes

@Markoutte Markoutte requested a review from CaelmBleidd October 4, 2022 10:06
@Markoutte Markoutte force-pushed the pelevin/747_Other_class_is_not_mocked_as_required branch from 7a7eeb8 to 35408f2 Compare October 4, 2022 10:07
@Markoutte Markoutte marked this pull request as ready for review October 4, 2022 10:07
Copy link
Member

@CaelmBleidd CaelmBleidd left a comment

Choose a reason for hiding this comment

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

LGTM


}

internal fun MethodId.toFuzzerMockable(block: suspend SequenceScope<Pair<MethodId, List<UtModel>>>.() -> Unit): FuzzerMockableMethodId {
Copy link
Member

Choose a reason for hiding this comment

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

Could you make a data class for these pairs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"Pair" is used as convenient and well-known construction like "key to value". Because this method is used by dsl I'd prefer to keep it this way.

Also, I have some doubt about using data classes for such small pairs. Maybe, typealias is more proper way in these cases?

constructorId.classId,
g.name,
g.returnType.id,
emptyList()
Copy link
Member

Choose a reason for hiding this comment

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

Please, use a named argument here

Comment on lines 164 to 165
setterAndGetter?.first,
setterAndGetter?.second,
Copy link
Member

Choose a reason for hiding this comment

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

And here (a data class). From a browser, it's not obvious which one is what.

values[index].add(model)
val mock = replaceToMock(model.model, description.shouldMock)
values[index].add(FuzzedValue(mock, model.createdBy).apply {
summary = model.summary
Copy link
Member

Choose a reason for hiding this comment

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

Why is it not a part of the constructor?

Comment on lines 31 to 59
fun replaceToMock(assembleModel: UtModel, shouldMock: (ClassId) -> Boolean): UtModel {
if (assembleModel !is UtAssembleModel) return assembleModel
if (shouldMock(assembleModel.classId)) {
return UtCompositeModel(assembleModel.id, assembleModel.classId, true).apply {
assembleModel.modificationsChain.forEach {
if (it is UtDirectSetFieldModel) {
fields[it.fieldId] = replaceToMock(it.fieldModel, shouldMock)
}
if (it is UtExecutableCallModel && it.executable is FuzzerMockableMethodId) {
(it.executable as FuzzerMockableMethodId).mock().forEach { (executionId, models) ->
mocks[executionId] = models.map { p -> replaceToMock(p, shouldMock) }
}
}
}
}
} else {
val models = assembleModel.modificationsChain.map { call ->
var mockedStatementModel: UtStatementModel? = null
if (call is UtDirectSetFieldModel) {
val mock = replaceToMock(call.fieldModel, shouldMock)
if (mock != call.fieldModel) {
mockedStatementModel = UtDirectSetFieldModel(call.instance, call.fieldId, mock)
}
} else if (call is UtExecutableCallModel) {
val params = call.params.map { m -> replaceToMock(m, shouldMock) }
if (params != call.params) {
mockedStatementModel = UtExecutableCallModel(call.instance, call.executable, params)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

As for me, it is hard to read because of missing spaces. My suggestion is something like:

fun replaceToMock(
    assembleModel: UtModel,
    shouldMock: (ClassId) -> Boolean
): UtModel = with(assembleModel) {
    if (this !is UtAssembleModel) return this

    if (shouldMock(classId)) {
        UtCompositeModel(id, classId, isMock = true).apply {
            modificationsChain.forEach {
                if (it is UtDirectSetFieldModel) {
                    fields[it.fieldId] = replaceToMock(it.fieldModel, shouldMock)
                }

                if (it is UtExecutableCallModel && it.executable is FuzzerMockableMethodId) {
                    (it.executable as FuzzerMockableMethodId).mock().forEach { (executionId, models) ->
                        mocks[executionId] = models.map { p -> replaceToMock(p, shouldMock) }
                    }
                }
            }
        }
    } else {
        val models = modificationsChain.map { call ->
            var mockedStatementModel: UtStatementModel? = null

            if (call is UtDirectSetFieldModel) {
                val mock = replaceToMock(call.fieldModel, shouldMock)

                if (mock != call.fieldModel) {
                    mockedStatementModel = UtDirectSetFieldModel(call.instance, call.fieldId, mock)
                }
            } else if (call is UtExecutableCallModel) {
                val params = call.params.map { m -> replaceToMock(m, shouldMock) }

                if (params != call.params) {
                    mockedStatementModel = UtExecutableCallModel(call.instance, call.executable, params)
                }
            }

            mockedStatementModel ?: call
        }

        with(assembleModel) {
            UtAssembleModel(id, classId, modelName, instantiationCall, origin) { models }
        }
    }
}

@Markoutte Markoutte force-pushed the pelevin/747_Other_class_is_not_mocked_as_required branch from a899cec to 23dc52a Compare October 11, 2022 12:35
@Markoutte Markoutte merged commit 1ba00d4 into main Oct 11, 2022
@Markoutte Markoutte deleted the pelevin/747_Other_class_is_not_mocked_as_required branch October 11, 2022 13:40
AbdullinAM pushed a commit to AbdullinAM/UTBotJava that referenced this pull request Oct 17, 2022
AbdullinAM pushed a commit to AbdullinAM/UTBotJava that referenced this pull request Oct 17, 2022
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

Successfully merging this pull request may close these issues.

Other class is not mocked as required
2 participants