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
Fix update rulesets - WINDUP-701 #643
Conversation
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): Failed Tests: 1org.jboss.windup.ui:windup-ui-tests: 1Test FAILed. |
549b344
to
5566774
Compare
Currently I am hitting |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): Failed Tests: 1org.jboss.windup.ui:windup-ui-tests: 1Test FAILed. |
Refer to this link for build results (access rights to CI server needed): |
1a89b1c
to
fffaabd
Compare
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
2b5c916
to
8596fd2
Compare
|
||
FileUtils.deleteDirectory(addonsDir.toFile()); | ||
FileUtils.deleteDirectory(libDir.toFile()); | ||
FileUtils.deleteDirectory(binDir.toFile()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these line will not work for Windows because Windows will not let you remove files that are in use or am I wrong?
Refer to this link for build results (access rights to CI server needed): Failed Tests: 4org.jboss.windup.reporting:windup-reporting-tests: 1org.jboss.windup.tests:windup-tests: 2
org.jboss.windup.ui:windup-ui-tests: 1Test FAILed. |
Refer to this link for build results (access rights to CI server needed): Failed Tests: 1org.jboss.windup.rules:windup-rulesets: 1Test FAILed. |
Refer to this link for build results (access rights to CI server needed): Failed Tests: 2org.jboss.windup.tests:windup-tests: 2
Test FAILed. |
Refer to this link for build results (access rights to CI server needed): Failed Tests: 1org.jboss.windup.rules:windup-rulesets: 1Test FAILed. |
Locally the tests all pass. |
Refer to this link for build results (access rights to CI server needed): Failed Tests: 3org.jboss.windup.reporting:windup-reporting-tests: 1org.jboss.windup.tests:windup-tests: 2
Test FAILed. |
Refer to this link for build results (access rights to CI server needed): Failed Tests: 1org.jboss.windup.rules:windup-rulesets: 1Test FAILed. |
The tests pass locally. |
retest |
Refer to this link for build results (access rights to CI server needed): Failed Tests: 1org.jboss.windup.rules:windup-rulesets: 1Test FAILed. |
Refer to this link for build results (access rights to CI server needed): |
3c26abf
to
ca77162
Compare
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
81114f6
to
87b2006
Compare
WINDUP-705 Fix automatic distribution updating - partial (shares some code with 701)
87b2006
to
a7ce016
Compare
Squashed |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): Failed Tests: 1org.jboss.windup.rules:windup-rulesets: 1Test FAILed. |
@@ -258,31 +257,32 @@ else if ("--generateCompletionData".equals(arg)) | |||
furnaceService.getFurnace().addRepository(AddonRepositoryMode.MUTABLE, new File(getUserWindupDir(), "addons")); | |||
} | |||
|
|||
if (command == null && updateRulesets && onlyRulesetsUpdateRequested(arguments)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if --updateRulesets is specified in addition to windup execution/analysis information? It currently looks like it will skip the ruleset update silently. That doesn't seem right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we probably need to be able to do both comments (e.g. allow multiple commands to be executed based on CLI values supplied.)
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Maybe I could move the RulesetsUpdater and DistributionUpdater to the same addon. And maybe join them to one class. |
Closed in favor of PR that includes this commit. |
…in hint as a parameterized section (windup#643)
I refactored the whole code and made the tests to work, and now I am going to check how it works practically.
The command will be removed. Instead it will be --updateRulesets, handled either in Bootstrap or as an ConfigurationOption and a rule, if that works.
I had to add this to the windup-ui-tests POM to prevent CNFEx,
but it's wrong since threre already is this one with forge-addon, only as provided which is then not picked up in tests (despite @AddonDependency(name = "org.jboss.forge.addon:maven") .)