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

fix: fix Maven source and test source directory resolving #3562

Merged
merged 4 commits into from
Sep 9, 2020

Conversation

Strum355
Copy link
Contributor

@Strum355 Strum355 commented Sep 2, 2020

By climbing the pom.xml hierarchy to find the nearest ancestor that declares the source directory rather than immediately falling back to src/(main|test)/java if not declared in this immediate pom.

Also added interpolating (project|pom|'').basedir which is not uncommonly found in projects.

@monperrus
Copy link
Collaborator

Cool! Would you add the corresponding test case? Thanks!

@Strum355 Strum355 marked this pull request as draft September 3, 2020 12:44
@Strum355 Strum355 marked this pull request as ready for review September 3, 2020 12:44
@Strum355
Copy link
Contributor Author

Strum355 commented Sep 7, 2020

Not sure why the test is failing, it works locally

@monperrus
Copy link
Collaborator

Thanks a lot for the test case + the fix. I guess the CI environment is slightly different or a file is missing?

…om.xml hierarchy to find the nearest ancestor that declares the source directory. Also added interpolating (project|pom|'').basedir
@Strum355 Strum355 force-pushed the better-pom branch 3 times, most recently from 76db1d3 to be9816f Compare September 9, 2020 11:09
@Strum355
Copy link
Contributor Author

Strum355 commented Sep 9, 2020

@monperrus fixed the test case, the folder was missing because there were no files 👍

Copy link
Collaborator

@monperrus monperrus left a comment

Choose a reason for hiding this comment

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

Thanks, we're almost there, see comment below

SpoonPom childModel = pomModel.getModules().get(0);
//contract: source directory is derived from parent pom.xml if not declared in the current
// (childModel) SpoonPom
assertEquals(expected, childModel.getSourceDirectories().get(0).getAbsolutePath(), expected);
Copy link
Collaborator

Choose a reason for hiding this comment

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

expected should be the second argument, not the last
you can remove the first one, the message, not relevant here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed according to the hover definition 👍
image

@monperrus monperrus changed the title Improved SpoonPom source and test source directory resolving fix: fix Maven source and test source directory resolving Sep 9, 2020
@monperrus monperrus merged commit ee739b5 into INRIA:master Sep 9, 2020
@monperrus
Copy link
Collaborator

Thanks a lot @Strum355

@monperrus monperrus mentioned this pull request Oct 14, 2020
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.

2 participants