search

mathworks / jenkins-matlab-plugin

This plugin enables you to run MATLAB® and Simulink® as part of your Jenkins™ build.
45 stars 54 forks source link

Files from '.matlab' folder are not deleted #332

Closed Vahila closed 1 month ago

Vahila commented 3 months ago

Files from '.matlab' folder are not deleted. This affects the code coverage results when user doesn't specify the source folder.

sameagen-MW commented 3 months ago

Confirmed to occur in dev_main

sameagen-MW commented 3 months ago

Hey Vahila, sorry that I'm just now getting to look into this. Ended up getting wrapped up into a couple of other things for the last few days.

To make sure that I understand the issue correctly, the temporary script that's generated in the temp folder to run tests is being included within code coverage results if the user doesn't specify the source directory?

I don't think that's a bug, as we can't delete the script before generating results because the script is what's actually running the tests and collecting the coverage.

@nikhilbhoski and @mcafaro, what are your thoughts here? Are there any measures that we can take to get around this like a way to always exclude the .matlab folder when collecting coverage?

sameagen-MW commented 3 months ago

It seems like we could do something strange like deleting the file from within itself once it's loaded into memory, but I think my preference would probably just be to add in the restriction that a folder(s) must be specified to collect code coverage.

I think that's probably untenable though, as it would introduce a backwards incompatibility. Hmm, I'm not a huge fan of getting into the habit of using self-destructing files if we can avoid it so maybe the best option is to just leave it, and if the user doesn't like the script showing up they can specify a source folder?

sameagen-MW commented 3 months ago

I don't know, I"m pretty conflicted here.

Vahila commented 3 months ago

Hey Vahila, sorry that I'm just now getting to look into this. Ended up getting wrapped up into a couple of other things for the last few days.

To make sure that I understand the issue correctly, the temporary script that's generated in the temp folder to run tests is being included within code coverage results if the user doesn't specify the source directory?

I don't think that's a bug, as we can't delete the script before generating results because the script is what's actually running the tests and collecting the coverage.

@nikhilbhoski and @mcafaro, what are your thoughts here? Are there any measures that we can take to get around this like a way to always exclude the .matlab folder when collecting coverage?

Hi Sam, yes, that is the issue here where the coverage metrics includes the files from '.matlab' folder when the user doesn't specify the source directory.

This would be an unexpected behavior from user perspective as the source folder option is optional and also this include files that are generated by the plugin and are not related to the user. So I think we need to handle it somehow(deleting the files or excluding the folder)

sameagen-MW commented 3 months ago

Yeah, that's a good point about the behavior being unexpected. I'll keep thinking about it, I'm not sure that I'm totally convinced about what to do yet, as I think collecting code coverage for the entire workspace can run into other issues as well and is probably bad practice.

In the meantime, I do think the cleanup can be enhanced since at the moment it's tied to Java shutdown hooks which isn't ideal if the JVM instance is long-lived. I'll get that updated while I think about the code coverage issue.

Vahila commented 1 month ago

This issue is resolved in 2.14.0 version