Add support for non-branch coverage goals in DynaMOSA#142
Add support for non-branch coverage goals in DynaMOSA#142Aditya-9215 wants to merge 14 commits intose2p:mainfrom
Conversation
LuKrO2011
left a comment
There was a problem hiding this comment.
Hi,
thank you for the pull request! Before requesting review for a non-draft PR, please make sure all CI checks pass. For running the checks locally, see CONTRIBUTING.md.
I assume you were working on #119 and this PR, once merged, closes #119.
|
Hi Lukas,
Thank you for your feedback!
I have addressed the CI issues (mypy and pre-commit), and all checks pass
locally now. The remaining workflow appears to require maintainer approval
to run.
This PR addresses issue #119 by extending DynaMOSA to support non-branch
coverage goals (e.g., line coverage), while preserving the existing
branch-based dependency handling.
Please let me know if any further changes are needed. I’d be happy to
refine the implementation.
Best regards,
Aditya
…On Mon, Mar 23, 2026 at 12:45 PM Lukas Krodinger ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Hi,
thank you for the pull request! Before requesting review for a non-draft
PR, please make sure all CI checks pass. For running the checks locally,
see CONTRIBUTING.md
<https://gitlab.infosun.fim.uni-passau.de/se2/pynguin/pynguin/-/blob/f270973c64250b52036441df063fe589167f4b24/CONTRIBUTING.md>
.
I assume you were working on #119
<#119> and this PR, once merged,
closes #119 <#119>.
—
Reply to this email directly, view it on GitHub
<#142?email_source=notifications&email_token=BON7X7CZQFJIWVLPK4YV4R34SDQADA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTGOJYHE3TMNRTGE32M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-3989766317>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BON7X7GYRZXTEOYUNLTJGPT4SDQADAVCNFSM6AAAAACW2PTVOGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTSOBZG43DMMZRG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
LuKrO2011
left a comment
There was a problem hiding this comment.
Hi,
thank you for your contribution. However, I can not accept the PR as it is, because of the following three reasons. This PR
- changes stuff unrelated to the issue
- removes active goal updating which breaks the idea of DynaMOSA
- does not convince me that the limitation mentioned in the issue is addressed (as there is no new test showing that this is fixed now)
|
Hi @LuKrO2011, Thanks for your feedback. I have addressed the issues:
Please let me know if any further changes are needed. |
LuKrO2011
left a comment
There was a problem hiding this comment.
Hi,
thank you for your updating your contribution. Unrelated changes to the issue have been removed successfully. However, there are still limitations:
- tests do not pass (again)
- does not convince me that the limitation mentioned in the issue is addressed (as there is no new test showing that this is fixed now)
Additionally, I question whether the proposed way of allowing for additional "non-branch" fitness functions is a good way to address the issue. While the original DynaMosa paper does not handle line coverage, generalizing from how the other coverage metrics (statement, branch and mutation coverage) are handled one should rather aim for using LineCoverageGoals instead.
| non_branch_fitness_functions: OrderedSet[ff.FitnessFunction] = OrderedSet() | ||
|
|
||
| for fit in fitness_functions: | ||
| assert isinstance(fit, bg.BranchCoverageTestFitness) |
There was a problem hiding this comment.
In the current implementation, DynaMOSA will not work if one fitness_functions is not BranchCoverageTestFitness. If this stays like this, we should still ensure that at least one of the fitness_functions is a BranchCoverageTestFitness.
| self._current_goals: OrderedSet[ff.FitnessFunction] = OrderedSet(self._graph.root_branches) | ||
|
|
||
| # Store non-branch goals separately (DO NOT activate yet) | ||
| self._non_branch_goals: OrderedSet[ff.FitnessFunction] = non_branch_fitness_functions |
There was a problem hiding this comment.
The name might cause confusion with CodeObjectGoals which are also "non-branch" goals but might belong to the self._current_goals.
If the logic stays like this, I am fine with this naming, but add a respective comment on what's the difference.
| # Add non-branch goals ONLY after all branch goals are covered | ||
| if len(self._archive.uncovered_goals) == 0: | ||
| added = False | ||
| for goal in self._non_branch_goals: | ||
| if goal not in self._current_goals: | ||
| self._current_goals.add(goal) | ||
| added = True | ||
|
|
||
| if added: | ||
| self._archive.add_goals(self._current_goals) # type: ignore[arg-type] |
There was a problem hiding this comment.
For simplicity, let's assume the second FitnessFunction is a LineCoverageTestFitness function.
First, such goals should probably be encoded as LineCoverageGoals.
Second, we might already be able to cover such goals before all branches are covered.
There was a problem hiding this comment.
Unit tests with heavy mocking are great to test some behaviour of your code in isolation. In this case it is tested, that initially "non-branch" goals are not active, which makes sense if it is intended as it is in this case.
However, this does not test other properties of the algorithm, such as what happens once all branch goals are covered.
Even if that is also covered with a unit test with heavy mocking, it is still not tested that the behaviour is the same for non-mocked stuff. In general, using a simple non-mocked example with a real archive, real goals and a real subject is preferrable here.
Even if all of that is added, I would still not be convinced that now DynaMOSA + LineCoverage works. This must be tested with an integration test.
|
Hi @LuKrO2011, Thank you for the detailed feedback — I really appreciate the time and clarity. I understand now that my current approach of introducing separate "non-branch" goals does not align well with the existing design of Pynguin. Your suggestion to instead work with LineCoverageGoals makes sense, and I see that it would integrate more naturally with the architecture. I also acknowledge that my current tests are too limited and rely heavily on mocking. I will work on improving this by:
Before proceeding with a redesign, I wanted to confirm: Thanks again for your guidance — this has been very helpful for my understanding. Best regards, |
|
Hi @Aditya-9215 , Let me add my two cents to the discussions, wearing my maintainer's goggles. First, we appreciate and value contributions to Pynguin. Supporting line coverage in DynaMOSA can be valuable, e.g. to also allow our checked-coverage implementation to be used with DynaMOSA (it is currently tied to MOSA). We are willing to incorporate contributions if they provide a value for Pynguin, its users, and also for us. Because all code comes with the cost of maintaining it in the longer term. Second, now on your specific contribution: in my understanding, adding line-coverage support to DynaMOSA should require little to no changes in the algorithm's implementation. You might need to generify certain types, e.g. What I can strongly recommend is to check the MOSA implementation. It used to work with line coverage. I have not validated this functionality recently but I am not aware of any obvious breaking changes from the top of my head. The main difference from MOSA to DynaMOSA is the introduction of the dynamic target selection (cf. the DynaMOSA paper). The idea here is to only incorporate those goals in the set of current optimisation goals that can be reached currently: in nested branches, you first need to cover the outer branch before the inner branch becomes a goal of interest; as long as you have not covered the outer branch, there is no chance to even reach, let alone covering, it! In order to make this reasonably work, you will then need to enhance the line-coverage fitness computation. This fitness is currently basically a binary fitness, i.e. it is One final thought on checking things in EvoSuite: sometimes it can be helpful because Pynguin is inspired very much by EvoSuite. However, EvoSuite is a huge and complex piece of software. In my experience it is often faster to take a pen and paper (or white/black board, whatever you have in reach and prefer) and sketch the idea until one fully understands how the solution should look like, e.g. from an algorithmic point of view. From there you can usually implement it in Pynguin quite easily. EvoSuite, on the other hand, requires you to (a) find the relevant parts of the code, (b) understand them, and (c) transfer them to Pynguin, which differs significantly in many parts. Looking forward to your implementation of line-coverage support for DynaMOSA! Best, |
|
Hi @stephanlukasczyk , Thank you for the detailed explanation — this clarifies a lot. I understand now that my previous approach of introducing separate handling for non-branch goals does not align with the intended design. I will rework the implementation to instead generalize DynaMOSA to operate on I will also:
I will revisit the MOSA implementation as suggested and align the design accordingly. Thanks again for the guidance! |
|
Hi, thank you for the detailed feedback and suggestions. I have updated the implementation to better align with the intended design:
I also added:
All tests and checks pass locally. I would appreciate your feedback on whether this direction now better addresses the original issue. Best regards, |
|
Dear @Aditya-9215 , I am sorry but I do not see how your changes in 516c14d target the items I have put on the table in my previous comment. Please excuse if I am wrong with my assumption, however, it is a gut feeling: are you working on this? Or are you only pasting whatever @LuKrO2011 and myself are writing into some LLM, and send us back the LLM's answer? I have this assumption because of several reasons: (a) the code changes we are seeing are very similar each time, (b) the way you structure you answers is very much what I know from coding agents (these bullet points describing what it did, and what it also did, things that are often not really reflected in the code), and (c) the emojis in the code's comments. I might be wrong and if I was I am deeply sorry about my accusations, but if I am right, it would be a waste of @LuKrO2011 's and my time. Because prompting an LLM is something that we could also do ourselves. That said, we are happy to help you (and we already provided you help in our previous comments), if we have the feeling we are working with a human being. However, we won't continue helping, if we have the feeling that we are only going in circles. Let me propose a strategy to continue this work:
Following this proposal should give you a deep understanding of what is necessary to achieve your own goal. It also assures that both we and you have the same understanding of the goal and the path towards the goal. Afterwards implementing the feature should be fairly straight-forward. Thank you very much for your understanding. Looking foward to hearing from you, |
|
Hi @stephanlukasczyk , thank you again for your feedback and for outlining a clear path forward. I have taken a step back and tried to rethink the problem from an architectural perspective instead of iterating directly on code. Based on my current understanding, I would like to propose the following approach before continuing further implementation:
I would appreciate your feedback on whether this design aligns with your expectations before I proceed further. Best regards, |
This PR extends DynaMOSA to support non-branch fitness functions, including line coverage.
Key improvements:
Testing:
Notes: