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

[MNG-6268] When a reactor build fails Maven should include -f (if used) in command line suggestion #398

Closed
wants to merge 1 commit into from

Conversation

michael-o
Copy link
Member

I don't know whether this should be included, but it is trivial to provide. I will let the community decide. If accepted someone should write an IT for that.

@rfscholte
Copy link
Contributor

Writing a unittest is much easier in this case and sufficient enough.

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

I agree that a unit test is enough here

@michael-o
Copy link
Member Author

Just scratching my head how the full boostrap with a unit test would work here since I'd need to analyze stdout/err.

@rfscholte
Copy link
Contributor

IIRC @mthmulders had a similar usecase recently

@mthmulders
Copy link
Contributor

IIRC @mthmulders had a similar usecase recently

Not sure if I understand correctly what you refer to. I recently had a contribution to Plexus Archiver where I wrote a CapturingLog. But given that this is SLF4J, it might be easier to go with something like SLF4J Test.

@michael-o
Copy link
Member Author

The SFL4J test looks promising, but wouldn't it be easier to do just an IT instead of going through a new component?

@rfscholte
Copy link
Contributor

Another option is to increase the visibility of logBuildResumeHint( String resumeBuildHint ) a bit, in the unittest making a new MavenCli with an override of this method. To me an IT is overkill for this check, as it is simple input/output validation, only the difficulty here is that it is writing directly the the logging framework. Better adjust the class to make it testable

@michael-o
Copy link
Member Author

Another option is to have no test at all since this change is straight forward. Pipe input to output unmodified as you can see.

@rfscholte
Copy link
Contributor

I'm fine with no test in this case.

@michael-o
Copy link
Member Author

The issue has been closed as wontfix. So closing this, but leaving the branch for a while.

@michael-o michael-o closed this Jan 1, 2021
@hboutemy hboutemy deleted the MNG-6268 branch April 5, 2021 16:22
gnodet pushed a commit to gnodet/maven that referenced this pull request Nov 4, 2024
Add additional cases as original test is not the full story.
Make sure tree is same even if pushed down a level (in Maven3 is not)

---

https://issues.apache.org/jira/browse/MNG-8347
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.

4 participants