Github user kirklund commented on a diff in the pull request:

    https://github.com/apache/geode/pull/429#discussion_r107046157
  
    --- Diff: 
geode-core/src/test/java/org/apache/geode/internal/JarDeployerIntegrationTest.java
 ---
    @@ -39,83 +44,63 @@
       @Rule
       public TemporaryFolder temporaryFolder = new TemporaryFolder();
     
    -  @Rule
    -  public RestoreSystemProperties restoreSystemProperties = new 
RestoreSystemProperties();
    +  JarDeployer jarDeployer;
     
       @Before
       public void setup() {
    -    System.setProperty("user.dir", 
temporaryFolder.getRoot().getAbsolutePath());
         classBuilder = new ClassBuilder();
    -    ClassPathLoader.setLatestToDefault();
    +    jarDeployer = new JarDeployer(temporaryFolder.getRoot());
       }
     
    -  @Test
    -  public void testDeployFileAndChange() throws Exception {
    -    final JarDeployer jarDeployer = new JarDeployer();
    +  private byte[] createJarWithClass(String className) throws IOException {
    +    String stringBuilder = "package integration.parent;" + "public class " 
+ className + " {}";
     
    -    // First deploy of the JAR file
    -    byte[] jarBytes = this.classBuilder.createJarFromName("ClassA");
    -    JarClassLoader jarClassLoader =
    -        jarDeployer.deploy(new String[] {"JarDeployerDUnit.jar"}, new 
byte[][] {jarBytes})[0];
    -    File deployedJar = new File(jarClassLoader.getFileCanonicalPath());
    +    return 
this.classBuilder.createJarFromClassContent("integration/parent/" + className,
    +        stringBuilder);
    +  }
     
    -    assertThat(deployedJar).exists();
    -    assertThat(deployedJar.getName()).contains("#1");
    -    assertThat(deployedJar.getName()).doesNotContain("#2");
    +  @Test
    +  public void testFileVersioning() throws IOException, 
ClassNotFoundException {
    +    String jarName = "JarDeployerIntegrationTest.jar";
     
    -    assertThat(ClassPathLoader.getLatest().forName("ClassA")).isNotNull();
    +    byte[] firstJarBytes = createJarWithClass("ClassA");
     
    -    assertThat(doesFileMatchBytes(deployedJar, jarBytes));
    +    // First deploy of the JAR file
    +    DeployedJar firstDeployedJar = 
jarDeployer.deployWithoutRegistering(jarName, firstJarBytes);
     
    -    // Now deploy an updated JAR file and make sure that the next version 
of the JAR file
    -    // was created and the first one was deleted.
    -    jarBytes = this.classBuilder.createJarFromName("ClassB");
    -    JarClassLoader newJarClassLoader =
    -        jarDeployer.deploy(new String[] {"JarDeployerDUnit.jar"}, new 
byte[][] {jarBytes})[0];
    -    File nextDeployedJar = new 
File(newJarClassLoader.getFileCanonicalPath());
    +    
assertThat(firstDeployedJar.getFile()).exists().hasBinaryContent(firstJarBytes);
    +    
assertThat(firstDeployedJar.getFile().getName()).contains(".v1.").doesNotContain(".v2.");
     
    -    assertThat(nextDeployedJar.exists());
    -    assertThat(nextDeployedJar.getName()).contains("#2");
    -    assertThat(doesFileMatchBytes(nextDeployedJar, jarBytes));
    +    // Now deploy an updated JAR file and make sure that the next version 
of the JAR file
    +    // was created
    +    byte[] secondJarBytes = createJarWithClass("ClassB");
     
    -    assertThat(ClassPathLoader.getLatest().forName("ClassB")).isNotNull();
    +    DeployedJar secondDeployedJar = 
jarDeployer.deployWithoutRegistering(jarName, secondJarBytes);
    +    File secondDeployedJarFile = new 
File(secondDeployedJar.getFileCanonicalPath());
     
    -    assertThatThrownBy(() -> ClassPathLoader.getLatest().forName("ClassA"))
    -        .isExactlyInstanceOf(ClassNotFoundException.class);
    +    
assertThat(secondDeployedJarFile).exists().hasBinaryContent(secondJarBytes);
    +    
assertThat(secondDeployedJarFile.getName()).contains(".v2.").doesNotContain(".v1.");
     
    -    
assertThat(jarDeployer.findSortedOldVersionsOfJar("JarDeployerDUnit.jar")).hasSize(1);
    +    File[] sortedOldJars = jarDeployer.findSortedOldVersionsOfJar(jarName);
    +    assertThat(sortedOldJars).hasSize(2);
    +    assertThat(sortedOldJars[0].getName()).contains(".v2.");
    +    assertThat(sortedOldJars[1].getName()).contains(".v1.");
         assertThat(jarDeployer.findDistinctDeployedJars()).hasSize(1);
       }
     
       @Test
    -  public void testDeployNoUpdateWhenNoChange() throws Exception {
    -    final JarDeployer jarDeployer = new JarDeployer();
    -
    -    // First deploy of the JAR file
    -    byte[] jarBytes = 
this.classBuilder.createJarFromName("JarDeployerDUnitDNUWNC");
    -    JarClassLoader jarClassLoader =
    -        jarDeployer.deploy(new String[] {"JarDeployerDUnit2.jar"}, new 
byte[][] {jarBytes})[0];
    -    File deployedJar = new File(jarClassLoader.getFileCanonicalPath());
    -
    -    assertThat(deployedJar).exists();
    -    assertThat(deployedJar.getName()).contains("#1");
    -    JarClassLoader newJarClassLoader =
    -        jarDeployer.deploy(new String[] {"JarDeployerDUnit2.jar"}, new 
byte[][] {jarBytes})[0];
    -    assertThat(newJarClassLoader).isNull();
    -  }
    -
    -  @Test
       public void testDeployToInvalidDirectory() throws IOException, 
ClassNotFoundException {
    --- End diff --
    
    Change this to "throws Exception" -- that's the standard best practice for 
a @Test method in JUnit.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to