You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/12/01 23:25:52 UTC

[GitHub] [geode] demery-pivotal opened a new pull request #5800: GEODE-8755: Write bundled jars file to correct dir

demery-pivotal opened a new pull request #5800:
URL: https://github.com/apache/geode/pull/5800


   BundledJarsJUnitTest once again writes the bundled_jars.txt file to the
   integrationTest dir, where the failure message instructs the user to
   look for it.
   
   Authored-by: Dale Emery <de...@vmware.com>


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] demery-pivotal commented on a change in pull request #5800: GEODE-8755: Write bundled jars file to correct dir

Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5800:
URL: https://github.com/apache/geode/pull/5800#discussion_r534343409



##########
File path: geode-assembly/src/integrationTest/java/org/apache/geode/BundledJarsJUnitTest.java
##########
@@ -67,7 +67,7 @@ public void verifyBundledJarsHaveNotChanged() throws IOException {
         sortedJars.entrySet().stream().map(entry -> removeVersion(entry.getKey()));
     Set<String> bundledJarNames = new TreeSet<>(lines.collect(Collectors.toSet()));
 
-    Files.write(Paths.get("bundled_jars.txt"), bundledJarNames);
+    Files.write(Paths.get("..", "bundled_jars.txt"), bundledJarNames);

Review comment:
       Can you think of way to identify the tests that have this problem?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] yozaner1324 commented on a change in pull request #5800: GEODE-8755: Write bundled jars file to correct dir

Posted by GitBox <gi...@apache.org>.
yozaner1324 commented on a change in pull request #5800:
URL: https://github.com/apache/geode/pull/5800#discussion_r534359215



##########
File path: geode-assembly/src/integrationTest/java/org/apache/geode/BundledJarsJUnitTest.java
##########
@@ -67,7 +67,7 @@ public void verifyBundledJarsHaveNotChanged() throws IOException {
         sortedJars.entrySet().stream().map(entry -> removeVersion(entry.getKey()));
     Set<String> bundledJarNames = new TreeSet<>(lines.collect(Collectors.toSet()));
 
-    Files.write(Paths.get("bundled_jars.txt"), bundledJarNames);
+    Files.write(Paths.get("..", "bundled_jars.txt"), bundledJarNames);

Review comment:
       The three mentioned tests are the only ones I'm aware of that write files that need to be accessed after the test runs. I searched for all other occurrences of "Files.write" and none seemed important. I'm not sure there is a better way to identify which files are important vs random temp files. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] yozaner1324 commented on a change in pull request #5800: GEODE-8755: Write bundled jars file to correct dir

Posted by GitBox <gi...@apache.org>.
yozaner1324 commented on a change in pull request #5800:
URL: https://github.com/apache/geode/pull/5800#discussion_r534338470



##########
File path: geode-assembly/src/integrationTest/java/org/apache/geode/BundledJarsJUnitTest.java
##########
@@ -67,7 +67,7 @@ public void verifyBundledJarsHaveNotChanged() throws IOException {
         sortedJars.entrySet().stream().map(entry -> removeVersion(entry.getKey()));
     Set<String> bundledJarNames = new TreeSet<>(lines.collect(Collectors.toSet()));
 
-    Files.write(Paths.get("bundled_jars.txt"), bundledJarNames);
+    Files.write(Paths.get("..", "bundled_jars.txt"), bundledJarNames);

Review comment:
       This change looks good, however, BundledJarsJUnitTest is not the only test that has this problem; AssemblyContentsIntegrationTest and GeodeDependencyJarIntegrationTest also need essentially the same change.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] demery-pivotal commented on a change in pull request #5800: GEODE-8755: Write bundled jars file to correct dir

Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5800:
URL: https://github.com/apache/geode/pull/5800#discussion_r534356301



##########
File path: geode-assembly/src/integrationTest/java/org/apache/geode/BundledJarsJUnitTest.java
##########
@@ -67,7 +67,7 @@ public void verifyBundledJarsHaveNotChanged() throws IOException {
         sortedJars.entrySet().stream().map(entry -> removeVersion(entry.getKey()));
     Set<String> bundledJarNames = new TreeSet<>(lines.collect(Collectors.toSet()));
 
-    Files.write(Paths.get("bundled_jars.txt"), bundledJarNames);
+    Files.write(Paths.get("..", "bundled_jars.txt"), bundledJarNames);

Review comment:
       I did a scan of the integration test output to spot this kind of file. I found AssemblyContentsIntegrationTest and GeodeDependencyJarIntegrationTest. I will fix those. If you know of others, let me know.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] demery-pivotal merged pull request #5800: GEODE-8755: Write content list files to integration test dir

Posted by GitBox <gi...@apache.org>.
demery-pivotal merged pull request #5800:
URL: https://github.com/apache/geode/pull/5800


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] demery-pivotal commented on a change in pull request #5800: GEODE-8755: Write bundled jars file to correct dir

Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5800:
URL: https://github.com/apache/geode/pull/5800#discussion_r534343409



##########
File path: geode-assembly/src/integrationTest/java/org/apache/geode/BundledJarsJUnitTest.java
##########
@@ -67,7 +67,7 @@ public void verifyBundledJarsHaveNotChanged() throws IOException {
         sortedJars.entrySet().stream().map(entry -> removeVersion(entry.getKey()));
     Set<String> bundledJarNames = new TreeSet<>(lines.collect(Collectors.toSet()));
 
-    Files.write(Paths.get("bundled_jars.txt"), bundledJarNames);
+    Files.write(Paths.get("..", "bundled_jars.txt"), bundledJarNames);

Review comment:
       Can you think of way to identify the tests that have this problem?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org