Add binary path to staging #842
Conversation
src/test/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStagingTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStagingTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStagingTest.java
Show resolved
Hide resolved
src/test/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStagingTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStagingTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStagingTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStaging.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStaging.java
Outdated
Show resolved
Hide resolved
...in/java/com/google/cloud/tools/appengine/configuration/AppYamlProjectStageConfiguration.java
Show resolved
Hide resolved
src/main/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStaging.java
Outdated
Show resolved
Hide resolved
loosebazooka
force-pushed
the
add-binary-path-to-staging
branch
from
719766d
to
bdf326e
Compare
Codecov Report
@@ Coverage Diff @@
## master #842 +/- ##
============================================
- Coverage 78.46% 78.16% -0.31%
- Complexity 610 613 +3
============================================
Files 96 96
Lines 2410 2427 +17
Branches 282 285 +3
============================================
+ Hits 1891 1897 +6
- Misses 411 421 +10
- Partials 108 109 +1
Continue to review full report at Codecov.
|
src/main/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStaging.java
Show resolved
Hide resolved
| Path file = appEngineDirectory.resolve("app.yaml"); | ||
| Files.write( | ||
| file, | ||
| "entrypoint: custom custom".getBytes(StandardCharsets.UTF_8), |
There was a problem hiding this comment.
Oh! Now I think probably entrypoint: should be a string list, not string? That is, getEntrypoint() should return List<String>?
Whatever it should be, I think it's worth testing things like entrypoint: (null) or entrypoint: [] (getEntryoint() returning null and/or an empty list).
There was a problem hiding this comment.
So entrypoint in the app.yaml is a "space separated string", but I think @ludoch can verify this?
There was a problem hiding this comment.
Huh, it does look like the yaml property is a string according to the doc.
There was a problem hiding this comment.
Can you add another test for entrypoint:?
There was a problem hiding this comment.
To verify that using a list fails?
There was a problem hiding this comment.
No, I just want to confirm entrypoint: returns null instead of an empty string.
Yeah it should be now, not sure what's going on with codecov |
| Path file = appEngineDirectory.resolve("app.yaml"); | ||
| Files.write( | ||
| file, | ||
| "entrypoint: custom custom".getBytes(StandardCharsets.UTF_8), |
There was a problem hiding this comment.
Can you add another test for entrypoint:?
* Allow binaries in java11 standard buildpath
part of #840
requires #839
Allows binaries in java11 build path (for example when building a quarkus native app)
Also changes the deprecated use of appYamlStaging builder in tests