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

Clean coverage after tests applying #349

Merged

Conversation

arksap2002
Copy link
Collaborator

Description of changes made

After applying tests to the project the coverage have to be removed.

Other notes

Closes #347

  • I have checked that I am merging into correct branch

@arksap2002 arksap2002 added bug Something isn't working Ready for review PR redy for review labels Sep 3, 2024
@pderakhshanfar
Copy link
Collaborator

Why do we need to clean coverage and remove all tests after application? What if the user wants to apply a set of tests and then work on other tests, and then apply them again?

@Hello-zoka
Copy link
Collaborator

Hello-zoka commented Oct 8, 2024

FYI: My approval is for the existing changes, in case that cleaning the coverage is suitable

Why do we need to clean coverage and remove all tests after application? What if the user wants to apply a set of tests and then work on other tests and then apply them again?

For your use case showing coverage seems to be useful, but there are a couple of ideological problems:

  • All the coverage is cleared when we run a new attempt of generation(previous tests are not showing after applying them to the test suite)
  • After applying tests, links from covered lines to the test that covers it become incorrect. So the coverage is not shown in the correct way
  • For such a development process, it will also be logical to have all the coverage shown, but we are not collecting such information from already existing tests.

All this led me to the conclusion that we have to either clear coverage and make it just a useful visualization of currently generated tests(as it is now) or refactor how we treat coverage, in general, to make it useful for understanding the not-covered part of the project.

I am voting for the first option, at least for now. @Vladislav0Art @pderakhshanfar @arksap2002 your opinion on it?

@pderakhshanfar
Copy link
Collaborator

My problem is about removing test cases.
If you,
1- After test generation click on Apply to test suite
2- click on cancel after path selector pop up.
3- all tests are gone (and not only coverage info)

@Hello-zoka
Copy link
Collaborator

Hello-zoka commented Oct 9, 2024

My problem is about removing test cases.

Oh, I didn't notice such behaviour, it obviously has to be fixed. I was thinking about the concept of deleting coverage in general

@arksap2002
Copy link
Collaborator Author

My problem is about removing test cases. If you, 1- After test generation click on Apply to test suite 2- click on cancel after path selector pop up. 3- all tests are gone (and not only coverage info)

This issue fixed here: #333
I am workign on it

@Hello-zoka Hello-zoka added changes requested Reviewer(s) left some comments that should be addressed by PR maintainer and removed Ready for review PR redy for review labels Oct 9, 2024
@arksap2002
Copy link
Collaborator Author

arksap2002 commented Oct 9, 2024

@pderakhshanfar fixed, could you, please, check it again

Copy link
Collaborator

@Hello-zoka Hello-zoka left a comment

Choose a reason for hiding this comment

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

It works for me now

@Vladislav0Art Vladislav0Art removed the changes requested Reviewer(s) left some comments that should be addressed by PR maintainer label Oct 10, 2024
@Vladislav0Art Vladislav0Art merged commit 2a243ff into development Oct 10, 2024
3 checks passed
@arksap2002 arksap2002 deleted the arksap2002/bugs/clean-coverage-after-tests-applying branch October 15, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Coverage is not cleaned after tests applying
4 participants