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

(#8) - Remove a result XML file if a source file no longer exist #74

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

pollyvolk
Copy link
Contributor

@pollyvolk pollyvolk commented Dec 1, 2022

#8
How it was:
A user runs Polystat with --files sources --tmp result, and there is a file sources/test.eo. The result of the analysis will be saved to result/test.xml. If the user renames or deletes the file sources/test.eo and runs the same command again, Polystat will print the previous correct result, taking it from result/test.xml, not throwing an exception.

After changes:
In the same case Polystat will throw a FileNotFoundException.
It throws an exception several times for each running Analysis.

@codecov-commenter
Copy link

codecov-commenter commented Dec 1, 2022

Codecov Report

Merging #74 (2aa39fb) into master (acc7c35) will increase coverage by 0.34%.
The diff coverage is 50.00%.

@@             Coverage Diff              @@
##             master      #74      +/-   ##
============================================
+ Coverage     53.96%   54.30%   +0.34%     
- Complexity       32       34       +2     
============================================
  Files             8        8              
  Lines           265      267       +2     
  Branches         24       25       +1     
============================================
+ Hits            143      145       +2     
+ Misses          119      118       -1     
- Partials          3        4       +1     
Impacted Files Coverage Δ
src/main/java/org/polystat/Program.java 91.30% <50.00%> (+0.82%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pollyvolk
Copy link
Contributor Author

@mximp Please, review

@mximp
Copy link
Contributor

mximp commented Dec 5, 2022

@pollyvolk please always place reference to an issue within PR's description.

@@ -68,8 +68,11 @@ public Program(final Path src, final Path tmp) {
public XML apply(final String locator) throws Exception {
final String[] parts = locator.split("\\.");
final String name = parts[1];
final Path xml = this.temp.resolve(String.format("%s.xml", name));
final Path src = this.sources.resolve(String.format("%s.eo", name));
final Path xml = this.temp.resolve(String.format("%s.xml", name)).toAbsolutePath();
Copy link
Contributor

Choose a reason for hiding this comment

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

@pollyvolk what caused the path to be changed to absolute? Were there issues with previous version of the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mximp You are right, this change is redundant, I will remove it

@mximp
Copy link
Contributor

mximp commented Dec 5, 2022

@yegor256 please merge

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.

None yet

3 participants