Skip to content

Commit

Permalink
Reverse the reversed error message #6290 (#6368)
Browse files Browse the repository at this point in the history
Shows list of files not found first before listing the directories searched.
At the same time I referenced the constant for RESOURCES_FRONTEND_DEFAULT
instead of hard-coding that path.

Added two new test cases to cover single- and multi-file exception messages.

Fixes #6290
  • Loading branch information
MichaelWMerritt authored and Johannes Eriksson committed Sep 3, 2019
1 parent 058a0e5 commit b139db2
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import com.vaadin.flow.server.Constants;
import org.apache.commons.io.FileUtils;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -265,16 +266,17 @@ && frontendFileExists(localModulePath)) {
}

if (!resourceNotFound.isEmpty()) {
String prefix = String.format(
"Failed to resolve the following files either:"
+ "%n · in the `%s` sources folder"
+ "%n · or as a `META-INF/resources/frontend` resource in some JAR.",
frontendDirectory.getPath());
String prefix = "Failed to find the following files: ";
String suffix = String.format(
"Please, double check that those files exist. If you use a custom directory "
"%n Locations searched were:"
+ "%n - `%s` in this project"
+ "%n - `%s` in included JARs"
+ "%n%n Please, double check that those files exist. If you use a custom directory "
+ "for your resource files instead of default "
+ "`frontend` folder then make sure you it's correctly configured "
+ "(e.g. set '%s' property)",
frontendDirectory.getPath(),
Constants.RESOURCES_FRONTEND_DEFAULT,
FrontendUtils.PARAM_FRONTEND_DIR);
throw new IllegalStateException(
notFoundMessage(resourceNotFound, prefix, suffix));
Expand Down Expand Up @@ -313,7 +315,7 @@ private void handleImports(String path, AbstractTheme theme,
} catch (IOException exception) {
LoggerFactory.getLogger(TaskUpdateImports.class)
.warn("Could not read file {}. Skipping "
+ "applyig theme for its imports", file.getPath(),
+ "applying theme for its imports", file.getPath(),
exception);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.Set;
import java.util.stream.Collectors;

import com.vaadin.flow.server.Constants;
import org.apache.commons.io.FileUtils;
import org.hamcrest.CoreMatchers;
import org.junit.After;
Expand All @@ -43,6 +44,7 @@
import org.junit.rules.TemporaryFolder;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
import org.mockito.internal.util.collections.Sets;
import org.slf4j.Logger;
import org.slf4j.impl.SimpleLogger;

Expand All @@ -52,8 +54,9 @@
import static com.vaadin.flow.server.frontend.FrontendUtils.IMPORTS_NAME;
import static com.vaadin.flow.server.frontend.FrontendUtils.NODE_MODULES;
import static com.vaadin.flow.server.frontend.FrontendUtils.WEBPACK_PREFIX_ALIAS;
import static org.junit.Assert.assertFalse;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertFalse;

public class NodeUpdateImportsTest extends NodeUpdateTestUtil {

Expand Down Expand Up @@ -105,9 +108,9 @@ Logger log() {
}
};

Assert.assertTrue(nodeModulesPath.mkdirs());
assertTrue(nodeModulesPath.mkdirs());
createExpectedImports(frontendDirectory, nodeModulesPath);
Assert.assertTrue(new File(nodeModulesPath,
assertTrue(new File(nodeModulesPath,
FLOW_NPM_PACKAGE_NAME + "ExampleConnector.js").exists());
}

Expand Down Expand Up @@ -139,12 +142,12 @@ public void getModuleLines_npmPackagesDontExist_logExplanation() {
boolean atLeastOneRemoved = false;
for (String imprt : getExpectedImports()) {
if (imprt.startsWith("@vaadin") && imprt.endsWith(".js")) {
Assert.assertTrue(resolveImportFile(nodeModulesPath,
assertTrue(resolveImportFile(nodeModulesPath,
nodeModulesPath, imprt).delete());
atLeastOneRemoved = true;
}
}
Assert.assertTrue(atLeastOneRemoved);
assertTrue(atLeastOneRemoved);
updater.execute();

ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
Expand All @@ -161,6 +164,67 @@ public void getModuleLines_npmPackagesDontExist_logExplanation() {
"In addition you may run `npm install` to fix `node_modules` tree structure.")));
}

@Test
public void getModuleLines_oneFrontendDependencyDoesntExist_throwExceptionAndlogExplanation() {
useMockLog = true;
Mockito.when(logger.isInfoEnabled()).thenReturn(true);

String fooFileName = "./foo.js";
assertFileRemoved(fooFileName, frontendDirectory);

try {
updater.execute();
Assert.fail("Execute should have failed with missing file");
} catch (IllegalStateException e) {
assertThat(e.getCause().getMessage(),
CoreMatchers.containsString(getFormattedFrontendErrorMessage(Sets.newSet(fooFileName))));
}

}

@Test
public void getModuleLines_multipleFrontendDependencyDoesntExist_throwExceptionAndlogExplanation() {
useMockLog = true;
Mockito.when(logger.isInfoEnabled()).thenReturn(true);

String localTemplateFileName = "./local-template.js";
String fooFileName = "./foo.js";

assertFileRemoved(localTemplateFileName, frontendDirectory);
assertFileRemoved(fooFileName, frontendDirectory);

try {
updater.execute();
Assert.fail("Execute should have failed with missing files");
} catch (IllegalStateException e) {
assertThat(e.getCause().getMessage(),
CoreMatchers.containsString(getFormattedFrontendErrorMessage(Sets.newSet(localTemplateFileName, fooFileName))));
}

}

private void assertFileRemoved(String fileName, File directory) {
assertTrue(String.format("File `%s` was not removed from, or does not exist in, `%s`",
fileName, directory),
resolveImportFile(directory, directory, fileName).delete());
}

private String getFormattedFrontendErrorMessage(Set<String> resourcesNotFound) {
String prefix = "Failed to find the following files: ";

String suffix = String.format(
"%n Locations searched were:"
+ "%n - `%s` in this project"
+ "%n - `%s` in included JARs"
+ "%n%n Please, double check that those files exist. If you use a custom directory "
+ "for your resource files instead of default "
+ "`frontend` folder then make sure you it's correctly configured "
+ "(e.g. set '%s' property)", frontendDirectory.getPath(), Constants.RESOURCES_FRONTEND_DEFAULT, FrontendUtils.PARAM_FRONTEND_DIR);

return String.format("%n%n %s%n - %s%n %s%n%n", prefix,
String.join("\n - ", resourcesNotFound), suffix);
}

@Test
public void should_UpdateMainJsFile() throws Exception {
List<String> expectedLines = new ArrayList<>(Arrays.asList(
Expand Down Expand Up @@ -228,7 +292,7 @@ public void should_UpdateMainJsFile() throws Exception {

@Test
public void should_ThrowException_WhenCssFileNotFound() {
Assert.assertTrue(resolveImportFile(frontendDirectory, nodeModulesPath,
assertTrue(resolveImportFile(frontendDirectory, nodeModulesPath,
"@vaadin/vaadin-mixed-component/bar.css").delete());
exception.expect(IllegalStateException.class);
updater.execute();
Expand Down

0 comments on commit b139db2

Please sign in to comment.