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 java invalid base dir #900

Merged
merged 2 commits into from
Apr 10, 2019
Merged

Fix java invalid base dir #900

merged 2 commits into from
Apr 10, 2019

Conversation

abrooksv
Copy link
Contributor

@abrooksv abrooksv commented Apr 9, 2019

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Description

Query project root from Maven/Gradle to determine the base directory for Java

When using Java handler based build, query Maven/Gradle for the project root.

Motivation and Context

Related Issue(s)

#868, #857

Testing

We now actually import the projects and then try to build them

Screenshots (if appropriate)

License

I confirm that my contribution is made under the terms of the Apache 2.0 license.

@abrooksv abrooksv requested a review from a team as a code owner April 9, 2019 21:48
@abrooksv
Copy link
Contributor Author

abrooksv commented Apr 9, 2019

Opps, need to rebase on to clean origin/master

@abrooksv abrooksv force-pushed the fixJavaInvalidBaseDir branch from cea4b6d to 32890c4 Compare April 9, 2019 21:55
@codecov-io
Copy link

codecov-io commented Apr 9, 2019

Codecov Report

Merging #900 into master will decrease coverage by 15.57%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master     #900       +/-   ##
=============================================
- Coverage     60.02%   44.44%   -15.58%     
+ Complexity      605      468      -137     
=============================================
  Files           159      159               
  Lines          5223     5231        +8     
  Branches        653      654        +1     
=============================================
- Hits           3135     2325      -810     
- Misses         1810     2685      +875     
+ Partials        278      221       -57
Flag Coverage Δ Complexity Δ
#integtest ? ?
#uitest ? ?
#unittest 44.44% <0%> (-0.07%) 468 <0> (ø)
Impacted Files Coverage Δ Complexity Δ
...etbrains/services/lambda/java/JavaLambdaBuilder.kt 5.26% <0%> (-76.56%) 1 <0> (-2)
...e/src/software/aws/toolkits/core/s3/BucketUtils.kt 0% <0%> (-100%) 0% <0%> (ø)
.../jetbrains/services/telemetry/MessageBusService.kt 0% <0%> (-100%) 0% <0%> (ø)
...tials/ExtensionPointCredentialsProviderRegistry.kt 0% <0%> (-100%) 0% <0%> (-3%)
...ns/services/lambda/python/PythonSamDebugSupport.kt 0% <0%> (-96.97%) 0% <0%> (-3%)
...s/jetbrains/ui/wizard/IntelliJSdkSelectionPanel.kt 0% <0%> (-85.72%) 0% <0%> (-7%)
...its/jetbrains/ui/wizard/SamInitSelectionPanel.java 0% <0%> (-85%) 0% <0%> (-16%)
...ains/services/lambda/python/PythonLambdaBuilder.kt 0% <0%> (-84.62%) 0% <0%> (-4%)
...etbrains/services/lambda/deploy/SamDeployDialog.kt 0% <0%> (-82.31%) 0% <0%> (-16%)
...etbrains/services/lambda/deploy/SamDeployView.java 0% <0%> (-79.49%) 0% <0%> (-5%)
... and 57 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb22788...60031a6. Read the comment docs.

.forceWhenUptodate()
.use(ProgressExecutionMode.MODAL_SYNC)

ExternalSystemUtil.refreshProjects(importSpecBuilder)
Copy link
Contributor

Choose a reason for hiding this comment

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

does this happen synchronously?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is controlled by the ProgressExecutionMode enum 2 lines up

assertThatThrownBy {
buildLambda(projectRule.module, handlerPsi, Runtime.JAVA8, "com.example.SomeClass")
}.isInstanceOf(IllegalStateException::class.java)
.hasMessageEndingWith("is not managed by Maven or Gradle")
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, as the exception message is managed by the localization properties, I'm wondering if it is appropriate to test the exception message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya it is fragile, but it does enforce the correct exception is thrown and not a random different IllegalState

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand you want to enforce the correct exception. It's better to fix the fragile test, but I'm fine with it too considering we are not expanding the localization in a short term.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can just use the message() call here as well. I'll make the change tomorrow

@abrooksv abrooksv force-pushed the fixJavaInvalidBaseDir branch from 32890c4 to 6ec659c Compare April 9, 2019 23:47
…for Java

* When using Java handler based build, query Maven/Gradle for the project root.
@abrooksv abrooksv force-pushed the fixJavaInvalidBaseDir branch from 6ec659c to 75bdbe0 Compare April 10, 2019 16:45
@abrooksv abrooksv merged commit 7006c44 into master Apr 10, 2019
@abrooksv abrooksv deleted the fixJavaInvalidBaseDir branch April 10, 2019 17:00
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