From 07bf137e661b232b8c9f990730b7d4f48e781783 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Tue, 9 Aug 2022 15:08:38 -0700 Subject: [PATCH 01/13] Basic changes to bubble up realtime progress information --- .../com/salesforce/messaging/CliMessager.java | 66 ++++++++++++---- .../com/salesforce/messaging/EventKey.java | 16 +++- .../com/salesforce/messaging/Message.java | 18 ++++- messages/EventKeyTemplates.js | 13 +++- .../com/salesforce/CliMessagerAppender.java | 75 +++++++++++++++++++ sfge/src/main/java/com/salesforce/Main.java | 6 ++ .../graph/symbols/PathScopeVisitor.java | 4 +- .../salesforce/graph/symbols/ScopeUtil.java | 5 +- .../metainfo/CustomSettingInfoCollector.java | 6 ++ .../metainfo/MetaInfoCollector.java | 2 + .../metainfo/VisualForceHandlerImpl.java | 6 ++ .../salesforce/rules/PathBasedRuleRunner.java | 13 +++- .../rules/ThreadableRuleExecutor.java | 13 +++- .../rules/ops/ProgressListener.java | 12 +++ .../rules/ops/ProgressListenerImpl.java | 22 ++++++ sfge/src/main/resources/log4j2.xml | 16 ++-- .../test/java/com/salesforce/TestUtil.java | 4 +- .../rules/PathBasedRuleRunnerTest.java | 4 +- src/lib/services/CommandLineSupport.ts | 32 +++++++- src/lib/services/OutputProcessor.ts | 41 +++++++--- src/lib/sfge/SfgeWrapper.ts | 7 ++ 21 files changed, 328 insertions(+), 53 deletions(-) create mode 100644 sfge/src/main/java/com/salesforce/CliMessagerAppender.java create mode 100644 sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java create mode 100644 sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java diff --git a/cli-messaging/src/main/java/com/salesforce/messaging/CliMessager.java b/cli-messaging/src/main/java/com/salesforce/messaging/CliMessager.java index bf0a0a34f..b96a53aec 100644 --- a/cli-messaging/src/main/java/com/salesforce/messaging/CliMessager.java +++ b/cli-messaging/src/main/java/com/salesforce/messaging/CliMessager.java @@ -4,6 +4,7 @@ import java.util.Arrays; import java.util.List; +import com.google.common.collect.Lists; import com.google.gson.Gson; public class CliMessager { @@ -12,8 +13,17 @@ public class CliMessager { // The END string lets us know when a message stops, which should prevent bugs involving multi-line output. private static final String END = "SFDX-END"; + private static final String REALTIME_START = "SFCA-REALTIME-START"; + private static final String REALTIME_END = "SFCA-REALTIME-END"; + + /* Deprecated: Don't maintain state in a class that's essentially used as a utility.*/ + @Deprecated private static final List MESSAGES = new ArrayList<>(); + /** + * Deprecated - switch to static invocation of {@link #postMessage(String, EventKey, String...)} + */ + @Deprecated public static CliMessager getInstance() { return LazyHolder.INSTANCE; } @@ -26,6 +36,7 @@ public static CliMessager getInstance() { * * @param exception to send to Typescript layer */ + @Deprecated public void addMessage(MessagePassableException exception) { final EventKey eventKey = exception.getEventKey(); addMessage( @@ -43,34 +54,59 @@ public void addMessage(MessagePassableException exception) { * @param eventKey EventKey to display to user * @param args String args passed to the EventKey to make the displayed message meaningful */ + @Deprecated public void addMessage(String internalLog, EventKey eventKey, String... args) { - // Developer error if eventKey was not added to exception and we'll get a bunch of NPEs - assert (eventKey != null); - // Confirm that the correct number of arguments for the message has been provided - // If this fails, this would be a developer error - assert (eventKey.getArgCount() == args.length); - - final Message message = new Message( - eventKey.getMessageKey(), - Arrays.asList(args), - internalLog, - eventKey.getMessageType(), - eventKey.getMessageHandler(), - eventKey.isVerbose()); - MESSAGES.add(message); + final Message message = createMessage(internalLog, eventKey, args); + MESSAGES.add(message); } - /** + /** + * Publish formatted stdout message to pass onto Typescript layer. + * Make sure EventKey is updated with messages/EventKeyTemplates.json + * and has correct properties in the enum. + * + * @param internalLog Information for internal use. Will be logged but not displayed to user + * @param eventKey EventKey to display to user + * @param args String args passed to the EventKey to make the displayed message meaningful + */ + public static void postMessage(String internalLog, EventKey eventKey, String... args) { + final Message message = createMessage(internalLog, eventKey, args); + final List messages = Lists.newArrayList(message); + + final String messageAsJson = new Gson().toJson(messages); + System.out.println(REALTIME_START + messageAsJson + REALTIME_END); + } + + private static Message createMessage(String internalLog, EventKey eventKey, String[] args) { + // Developer error if eventKey was not added to exception and we'll get a bunch of NPEs + assert (eventKey != null); + // Confirm that the correct number of arguments for the message has been provided + // If this fails, this would be a developer error + assert (eventKey.getArgCount() == args.length); + + final Message message = new Message( + eventKey.getMessageKey(), + Arrays.asList(args), + internalLog, + eventKey.getMessageType(), + eventKey.getMessageHandler(), + eventKey.isVerbose()); + return message; + } + + /** * Convert all messages stored by the instance into a JSON-formatted string, enclosed in the start and end strings. * Java code can use this method to log the messages to console, and TypeScript code can seek the start and stop * strings to get an array of messages that can be deserialized. * @return */ + @Deprecated public String getAllMessagesWithFormatting() { final String messagesAsJson = getMessagesAsJson(); return START + messagesAsJson + END; } + @Deprecated private String getMessagesAsJson() { return new Gson().toJson(MESSAGES); } diff --git a/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java b/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java index a09f07269..adfee16e1 100644 --- a/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java +++ b/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java @@ -3,6 +3,8 @@ public enum EventKey { // MAKE SURE `messageKey` OF EVERY VALUE ADDED HERE HAS AN ENTRY IN 'messages/EventKeyTemplates.js'! + + /** PMD-CATALOGER RELATED **/ INFO_GENERAL_INTERNAL_LOG("info.generalInternalLog", 1, MessageType.INFO, MessageHandler.INTERNAL, true), WARNING_INVALID_CAT_SKIPPED("warning.invalidCategorySkipped", 1, MessageType.WARNING, MessageHandler.UX, true), WARNING_INVALID_RULESET_SKIPPED("warning.invalidRulesetSkipped", 1, MessageType.WARNING, MessageHandler.UX, true), @@ -20,8 +22,20 @@ public enum EventKey { ERROR_EXTERNAL_RECURSION_LIMIT("error.external.recursionLimitReached", 2, MessageType.ERROR, MessageHandler.UX, false), ERROR_EXTERNAL_XML_NOT_READABLE("error.external.xmlNotReadable", 2, MessageType.ERROR, MessageHandler.UX, false), ERROR_EXTERNAL_XML_NOT_PARSABLE("error.external.xmlNotParsable", 2, MessageType.ERROR, MessageHandler.UX, false), + + + + + /** SFGE RELATED **/ + INFO_GENERAL("info.sfgeInfoLog", 1, MessageType.INFO, MessageHandler.UX, true), + INFO_META_INFO_COLLECTED("info.sfgeMetaInfoCollected", 2, MessageType.INFO, MessageHandler.UX, true), + INFO_BEGIN_PATH_CREATION("info.sfgeBeginPathCreation", 0, MessageType.INFO, MessageHandler.UX, true), + INFO_END_PATH_CREATION("info.sfgeEndPathCreation", 1, MessageType.INFO, MessageHandler.UX, false), + INFO_PATH_ANALYSIS_PROGRESS("info.sfgePathAnalysisProgress", 2, MessageType.INFO, MessageHandler.UX, true), + WARNING_GENERAL("warning.sfgeWarnLog", 1, MessageType.WARNING, MessageHandler.UX, true), WARNING_MULTIPLE_METHOD_TARGET_MATCHES("warning.multipleMethodTargetMatches", 3, MessageType.WARNING, MessageHandler.UX, false), - WARNING_NO_METHOD_TARGET_MATCHES("warning.noMethodTargetMatches", 2, MessageType.WARNING, MessageHandler.UX, false); + WARNING_NO_METHOD_TARGET_MATCHES("warning.noMethodTargetMatches", 2, MessageType.WARNING, MessageHandler.UX, false), + ERROR_GENERAL("error.internal.sfgeErrorLog", 1, MessageType.ERROR, MessageHandler.UX, false); final String messageKey; final int argCount; diff --git a/cli-messaging/src/main/java/com/salesforce/messaging/Message.java b/cli-messaging/src/main/java/com/salesforce/messaging/Message.java index 20ba93ae0..4a71e20cb 100644 --- a/cli-messaging/src/main/java/com/salesforce/messaging/Message.java +++ b/cli-messaging/src/main/java/com/salesforce/messaging/Message.java @@ -6,7 +6,7 @@ public class Message { final private String messageKey; final private List args; - final private String internalLog; + final private String internalLog; final private MessageType type; final private MessageHandler handler; final private boolean verbose; @@ -34,6 +34,22 @@ public String getInternalLog() { return internalLog; } + public MessageType getType() { + return type; + } + + public MessageHandler getHandler() { + return handler; + } + + public boolean isVerbose() { + return verbose; + } + + public long getTime() { + return time; + } + enum MessageHandler { UX, INTERNAL diff --git a/messages/EventKeyTemplates.js b/messages/EventKeyTemplates.js index a5d0f3a48..3011303a4 100644 --- a/messages/EventKeyTemplates.js +++ b/messages/EventKeyTemplates.js @@ -7,7 +7,12 @@ module.exports = { "customEslintHeadsUp": "About to run Eslint with custom config in %s. Please make sure your current directory has all the required NPM dependencies.", "customPmdHeadsUp": "About to run PMD with custom config in %s. Please make sure that any custom rule references have already been added to the plugin through scanner:rule:add command.", "pmdRuleSkipped": "Omitting results for PMD rule \"%s\". Reason: %s.", - "unmatchedPathExtensionCpd": "Path extensions for the following files will not be processed by CPD: %s" + "unmatchedPathExtensionCpd": "Path extensions for the following files will not be processed by CPD: %s", + "sfgeInfoLog": "%s", + "sfgeMetaInfoCollected": "Loaded %s %s", + "sfgeBeginPathCreation": "Building paths across source code.", + "sfgeEndPathCreation": "Completed building paths across source code. Count of paths detected: %s", + "sfgePathAnalysisProgress": "Analyzed %s of %s paths." }, "warning": { "invalidCategorySkipped": "Cataloger skipped invalid PMD Category file '%s'.", @@ -20,7 +25,8 @@ module.exports = { "unexpectedPmdNodeType": "Encountered unexpected PMD node of type '%s'", "multipleMethodTargetMatches": "Total of %s methods in file %s matched name #%s", "noMethodTargetMatches": "No methods in file %s matched name #%s()", - "pmdConfigError": "PMD failed to evaluate rule '%s'. Message: %s" + "pmdConfigError": "PMD failed to evaluate rule '%s'. Message: %s", + "sfgeWarnLog": "%s" }, "error": { "internal": { @@ -28,7 +34,8 @@ module.exports = { "mainInvalidArgument": "INTERNAL ERROR: Invalid arguments passed to Main. Details: %s. Please log an issue with us at github.com/forcedotcom/sfdx-scanner.", "jsonWriteFailed": "INTERNAL ERROR: Failed to write JSON to file: %s. Please log an issue with us at github.com/forcedotcom/sfdx-scanner.", "classpathDoesNotExist": "INTERNAL ERROR: Path does not exist: %s. Please log an issue with us at github.com/forcedotcom/sfdx-scanner.", - "xmlMissingInClasspath": "INTERNAL ERROR: XML resource [%s] found in jar, but not in Classpath. Please log an issue with us at github.com/forcedotcom/sfdx-scanner." + "xmlMissingInClasspath": "INTERNAL ERROR: XML resource [%s] found in jar, but not in Classpath. Please log an issue with us at github.com/forcedotcom/sfdx-scanner.", + "sfgeErrorLog": "%s" }, "external": { "errorMessageAbove": "Please see error details displayed above.", diff --git a/sfge/src/main/java/com/salesforce/CliMessagerAppender.java b/sfge/src/main/java/com/salesforce/CliMessagerAppender.java new file mode 100644 index 000000000..d8cbf1c1f --- /dev/null +++ b/sfge/src/main/java/com/salesforce/CliMessagerAppender.java @@ -0,0 +1,75 @@ +package com.salesforce; + +import com.salesforce.messaging.CliMessager; +import com.salesforce.messaging.EventKey; +import java.io.Serializable; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.Appender; +import org.apache.logging.log4j.core.Core; +import org.apache.logging.log4j.core.Filter; +import org.apache.logging.log4j.core.Layout; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.logging.log4j.core.config.plugins.Plugin; +import org.apache.logging.log4j.core.config.plugins.PluginAttribute; +import org.apache.logging.log4j.core.config.plugins.PluginElement; +import org.apache.logging.log4j.core.config.plugins.PluginFactory; +import org.apache.logging.log4j.core.layout.PatternLayout; + +/** + * Custom log4j2 appender to send logs as realtime events through {@link CliMessager}. This helps + * streamline logs displayed to commandline users. Invoked from log4j.xml. + */ +@Plugin( + name = "CliMessagerAppender", + category = Core.CATEGORY_NAME, + elementType = Appender.ELEMENT_TYPE) +public class CliMessagerAppender extends AbstractAppender { + + @PluginFactory + public static CliMessagerAppender createAppender( + @PluginAttribute("name") String name, + @PluginElement("Layout") Layout layout, + @PluginElement("Filter") final Filter filter) { + if (name == null) { + LOGGER.error("There is no name provided for MyCustomAppender"); + return null; + } + if (layout == null) { + layout = PatternLayout.createDefaultLayout(); + } + return new CliMessagerAppender(name, filter, layout, true); + } + + protected CliMessagerAppender( + String name, + Filter filter, + Layout layout, + final boolean ignoreExceptions) { + super(name, filter, layout, ignoreExceptions, null); + } + + @Override + public void append(LogEvent event) { + Level level = event.getLevel(); + if (Level.INFO.equals(level)) { + CliMessager.postMessage("SFGE Info log", EventKey.INFO_GENERAL, getEventMessage(event)); + } else if (Level.WARN.equals(level)) { + CliMessager.postMessage("SFGE Warn", EventKey.WARNING_GENERAL, getEventMessage(event)); + } else if (Level.ERROR.equals(level) || Level.FATAL.equals(level)) { + CliMessager.postMessage( + "SFGE Error/Fatal", EventKey.ERROR_GENERAL, getEventMessage(event)); + } else { + // TODO: revisit + error( + String.format( + "Unable to log less than INFO level [{}]: {}", + event.getLevel(), + getEventMessage(event))); + } + } + + private String getEventMessage(LogEvent event) { + return event.getMessage().getFormattedMessage(); + } +} diff --git a/sfge/src/main/java/com/salesforce/Main.java b/sfge/src/main/java/com/salesforce/Main.java index 3b850f895..539160146 100644 --- a/sfge/src/main/java/com/salesforce/Main.java +++ b/sfge/src/main/java/com/salesforce/Main.java @@ -8,6 +8,7 @@ import com.salesforce.exception.UserActionException; import com.salesforce.graph.ops.GraphUtil; import com.salesforce.messaging.CliMessager; +import com.salesforce.messaging.EventKey; import com.salesforce.metainfo.MetaInfoCollector; import com.salesforce.metainfo.MetaInfoCollectorProvider; import com.salesforce.rules.AbstractRule; @@ -172,6 +173,11 @@ private void collectMetaInfo(CliArgParser.ExecuteArgParser eap) { for (MetaInfoCollector collector : allCollectors) { collector.loadProjectFiles(eap.getProjectDirs()); + CliMessager.postMessage( + "Meta information collected", + EventKey.INFO_META_INFO_COLLECTED, + String.valueOf(collector.getMetaInfoCollected().size()), + collector.getMetaInfoTypeName()); } } diff --git a/sfge/src/main/java/com/salesforce/graph/symbols/PathScopeVisitor.java b/sfge/src/main/java/com/salesforce/graph/symbols/PathScopeVisitor.java index 00e3d8b2e..15007bf5e 100644 --- a/sfge/src/main/java/com/salesforce/graph/symbols/PathScopeVisitor.java +++ b/sfge/src/main/java/com/salesforce/graph/symbols/PathScopeVisitor.java @@ -388,7 +388,7 @@ public Optional> getApexValue(VariableExpressionVertex var) { if (LOGGER.isWarnEnabled()) { LOGGER.warn( "TODO: PathScopeVisitor.getApexValue() can currently only support chains of length 2 or lower. keySequence=" - + keys); + + Arrays.toString(keys)); } return Optional.empty(); } @@ -600,7 +600,7 @@ public Optional getImplementingScope( if (LOGGER.isWarnEnabled()) { LOGGER.warn( "TODO: PathScopeVisitor.getApexValue() can currently only support chains of length 2 or lower. keySequence=" - + keys); + + Arrays.toString(keys)); } return Optional.empty(); } diff --git a/sfge/src/main/java/com/salesforce/graph/symbols/ScopeUtil.java b/sfge/src/main/java/com/salesforce/graph/symbols/ScopeUtil.java index 42a8cef79..f48c204a1 100644 --- a/sfge/src/main/java/com/salesforce/graph/symbols/ScopeUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/symbols/ScopeUtil.java @@ -11,6 +11,7 @@ import com.salesforce.graph.vertex.LiteralExpressionVertex; import com.salesforce.graph.vertex.PrefixExpressionVertex; import com.salesforce.graph.vertex.VariableExpressionVertex; +import java.util.Arrays; import java.util.Optional; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -151,8 +152,8 @@ public static Optional> getDefaultPropertyValue( if (keys.length > 2) { if (LOGGER.isWarnEnabled()) { LOGGER.warn( - "TODO: PathScopeVisitor.getApexValue() can currently only support chains of length 2 or lower. keySequence=" - + keys); + "PathScopeVisitor.getApexValue() can currently only support chains of length 2 or lower. keySequence=" + + Arrays.toString(keys)); } } else if (result instanceof ObjectProperties) { final ObjectProperties objectProperties = (ObjectProperties) result; diff --git a/sfge/src/main/java/com/salesforce/metainfo/CustomSettingInfoCollector.java b/sfge/src/main/java/com/salesforce/metainfo/CustomSettingInfoCollector.java index 8231716c9..342e2a6f4 100644 --- a/sfge/src/main/java/com/salesforce/metainfo/CustomSettingInfoCollector.java +++ b/sfge/src/main/java/com/salesforce/metainfo/CustomSettingInfoCollector.java @@ -24,6 +24,7 @@ public class CustomSettingInfoCollector extends XmlMetaInfoCollector { private static final String CUSTOM_SETTING_PATTERN_STRING = "(.*)" + OBJECT_FILE_NAME_PATTERN; private static final Pattern CUSTOM_SETTING_PATTERN = Pattern.compile(CUSTOM_SETTING_PATTERN_STRING, Pattern.CASE_INSENSITIVE); + private static final String META_INFO_TYPE_NAME = "Custom Settings"; @Override HashSet getPathPatterns() { @@ -88,6 +89,11 @@ private Optional getCustomSettingName(Path path) { return Optional.empty(); } + @Override + public String getMetaInfoTypeName() { + return META_INFO_TYPE_NAME; + } + protected static final class LazyHolder { // Postpone initialization until first use. protected static final CustomSettingInfoCollector INSTANCE = diff --git a/sfge/src/main/java/com/salesforce/metainfo/MetaInfoCollector.java b/sfge/src/main/java/com/salesforce/metainfo/MetaInfoCollector.java index 5b214c2bc..7038087aa 100644 --- a/sfge/src/main/java/com/salesforce/metainfo/MetaInfoCollector.java +++ b/sfge/src/main/java/com/salesforce/metainfo/MetaInfoCollector.java @@ -19,6 +19,8 @@ public interface MetaInfoCollector { */ TreeSet getMetaInfoCollected(); + String getMetaInfoTypeName(); + /** Thrown when project files cannot be properly loaded/processed. */ final class MetaInfoLoadException extends SfgeRuntimeException { MetaInfoLoadException(String msg) { diff --git a/sfge/src/main/java/com/salesforce/metainfo/VisualForceHandlerImpl.java b/sfge/src/main/java/com/salesforce/metainfo/VisualForceHandlerImpl.java index 53def4dba..fec93870a 100644 --- a/sfge/src/main/java/com/salesforce/metainfo/VisualForceHandlerImpl.java +++ b/sfge/src/main/java/com/salesforce/metainfo/VisualForceHandlerImpl.java @@ -15,6 +15,7 @@ public class VisualForceHandlerImpl extends AbstractMetaInfoCollector { private static final Logger LOGGER = LogManager.getLogger(VisualForceHandlerImpl.class); + private static final String META_INFO_TYPE_NAME = "Apex Controller entry points"; @Override protected TreeSet getAcceptedExtensions() { @@ -213,6 +214,11 @@ private void addReferencedNames(String concatenatedNames) { } } + @Override + public String getMetaInfoTypeName() { + return META_INFO_TYPE_NAME; + } + protected static final class LazyHolder { // Postpone initialization until first use. protected static final VisualForceHandlerImpl INSTANCE = new VisualForceHandlerImpl(); diff --git a/sfge/src/main/java/com/salesforce/rules/PathBasedRuleRunner.java b/sfge/src/main/java/com/salesforce/rules/PathBasedRuleRunner.java index 816f3ada0..63626c6e4 100644 --- a/sfge/src/main/java/com/salesforce/rules/PathBasedRuleRunner.java +++ b/sfge/src/main/java/com/salesforce/rules/PathBasedRuleRunner.java @@ -10,6 +10,7 @@ import com.salesforce.graph.vertex.SFVertex; import com.salesforce.rules.fls.apex.operations.FlsViolationInfo; import com.salesforce.rules.fls.apex.operations.FlsViolationMessageUtil; +import com.salesforce.rules.ops.ProgressListener; import java.util.HashSet; import java.util.List; import java.util.Optional; @@ -28,12 +29,18 @@ public class PathBasedRuleRunner { /** Set that holds the violations found by this rule execution. */ private final Set violations; + private final ProgressListener progressListener; + public PathBasedRuleRunner( - GraphTraversalSource g, List rules, MethodVertex methodVertex) { + GraphTraversalSource g, + List rules, + MethodVertex methodVertex, + ProgressListener progressListener) { this.g = g; this.rules = rules; this.methodVertex = methodVertex; this.violations = new HashSet<>(); + this.progressListener = progressListener; } /** @@ -45,8 +52,10 @@ public Set runRules() { // Build configuration to define how apex paths will be expanded final ApexPathExpanderConfig expanderConfig = getApexPathExpanderConfig(); + progressListener.initializingPathCreation(); // Get all the paths that originate in the entry point final List paths = getPaths(expanderConfig); + progressListener.identifiedPaths(paths); // Execute rules on the paths found executeRulesOnPaths(paths); @@ -81,6 +90,8 @@ private void executeRulesOnPaths(List paths) { // Iterate over all vertices in the path... for (ApexPathVertexMetaInfo.PredicateMatch predicateMatch : path.getApexPathMetaInfo().get().getAllMatches()) { + progressListener.pickedNewPathForAnalysis(path); + AbstractPathBasedRule rule = (AbstractPathBasedRule) predicateMatch.getPredicate(); BaseSFVertex vertex = predicateMatch.getPathVertex().getVertex(); diff --git a/sfge/src/main/java/com/salesforce/rules/ThreadableRuleExecutor.java b/sfge/src/main/java/com/salesforce/rules/ThreadableRuleExecutor.java index 0ff506365..ef4aa99e3 100644 --- a/sfge/src/main/java/com/salesforce/rules/ThreadableRuleExecutor.java +++ b/sfge/src/main/java/com/salesforce/rules/ThreadableRuleExecutor.java @@ -5,6 +5,8 @@ import com.salesforce.graph.JustInTimeGraphProvider; import com.salesforce.graph.ops.LogUtil; import com.salesforce.graph.vertex.MethodVertex; +import com.salesforce.rules.ops.ProgressListener; +import com.salesforce.rules.ops.ProgressListenerImpl; import java.util.List; import java.util.Set; import java.util.Timer; @@ -78,8 +80,9 @@ private static int submitRunners( CompletionService> completionService, List submissions) { int submissionCount = 0; + final ProgressListener progressListener = new ProgressListenerImpl(); for (ThreadableRuleSubmission submission : submissions) { - completionService.submit(new CallableExecutor(submission)); + completionService.submit(new CallableExecutor(submission, progressListener)); submissionCount += 1; } if (LOGGER.isInfoEnabled()) { @@ -111,10 +114,13 @@ private static Set waitForCallable( private static class CallableExecutor implements Callable> { private final ThreadableRuleSubmission submission; + private final ProgressListener progressListener; private boolean timedOut; - public CallableExecutor(ThreadableRuleSubmission submission) { + public CallableExecutor( + ThreadableRuleSubmission submission, ProgressListener progressListener) { this.submission = submission; + this.progressListener = progressListener; } public Set call() { @@ -151,7 +157,8 @@ public void run() { new PathBasedRuleRunner( submission.getGraph(), submission.getRules(), - submission.getPathEntry()) + submission.getPathEntry(), + progressListener) .runRules()); timer.cancel(); } catch (StackOverflowError | Exception ex) { diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java new file mode 100644 index 000000000..8763ad9cc --- /dev/null +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java @@ -0,0 +1,12 @@ +package com.salesforce.rules.ops; + +import com.salesforce.graph.ApexPath; +import java.util.List; + +public interface ProgressListener { + void initializingPathCreation(); + + void identifiedPaths(List paths); + + void pickedNewPathForAnalysis(ApexPath path); +} diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java new file mode 100644 index 000000000..b76cdc30d --- /dev/null +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java @@ -0,0 +1,22 @@ +package com.salesforce.rules.ops; + +import com.salesforce.graph.ApexPath; +import com.salesforce.messaging.CliMessager; +import com.salesforce.messaging.EventKey; +import java.util.List; + +public class ProgressListenerImpl implements ProgressListener { + + @Override + public void initializingPathCreation() { + CliMessager.postMessage("", EventKey.INFO_BEGIN_PATH_CREATION); + } + + @Override + public void identifiedPaths(List paths) { + CliMessager.postMessage("", EventKey.INFO_END_PATH_CREATION, String.valueOf(paths.size())); + } + + @Override + public void pickedNewPathForAnalysis(ApexPath path) {} +} diff --git a/sfge/src/main/resources/log4j2.xml b/sfge/src/main/resources/log4j2.xml index cd26f44bb..f69d0b0f7 100644 --- a/sfge/src/main/resources/log4j2.xml +++ b/sfge/src/main/resources/log4j2.xml @@ -1,7 +1,10 @@ - + + + + @@ -16,38 +19,33 @@ - + - + - - - - - - + diff --git a/sfge/src/test/java/com/salesforce/TestUtil.java b/sfge/src/test/java/com/salesforce/TestUtil.java index a14d208ed..683fd1749 100644 --- a/sfge/src/test/java/com/salesforce/TestUtil.java +++ b/sfge/src/test/java/com/salesforce/TestUtil.java @@ -38,6 +38,7 @@ import com.salesforce.rules.PathBasedRuleRunner; import com.salesforce.rules.StaticRule; import com.salesforce.rules.Violation; +import com.salesforce.rules.ops.ProgressListenerImpl; import java.io.File; import java.io.IOException; import java.lang.reflect.Method; @@ -311,7 +312,8 @@ public static List getViolations( final MethodVertex methodVertex = getMethodVertex(g, definingType, methodName); final PathBasedRuleRunner ruleRunner = - new PathBasedRuleRunner(g, Lists.newArrayList(rule), methodVertex); + new PathBasedRuleRunner( + g, Lists.newArrayList(rule), methodVertex, new ProgressListenerImpl()); final List violations = new ArrayList<>(ruleRunner.runRules()); return violations; diff --git a/sfge/src/test/java/com/salesforce/rules/PathBasedRuleRunnerTest.java b/sfge/src/test/java/com/salesforce/rules/PathBasedRuleRunnerTest.java index c05c3cf17..8aa6abef8 100644 --- a/sfge/src/test/java/com/salesforce/rules/PathBasedRuleRunnerTest.java +++ b/sfge/src/test/java/com/salesforce/rules/PathBasedRuleRunnerTest.java @@ -4,6 +4,7 @@ import com.salesforce.TestUtil; import com.salesforce.graph.vertex.MethodVertex; +import com.salesforce.rules.ops.ProgressListenerImpl; import java.util.Collections; import java.util.List; import java.util.Set; @@ -42,7 +43,8 @@ public void pathsWithoutInterestingVerticesAreIgnored() { Collections.singletonList(ApexFlsViolationRule.getInstance()); // Define a PathBasedRuleRunner to apply the rule against the method vertex. - PathBasedRuleRunner runner = new PathBasedRuleRunner(g, rules, methodVertex); + PathBasedRuleRunner runner = + new PathBasedRuleRunner(g, rules, methodVertex, new ProgressListenerImpl()); Set violations = runner.runRules(); MatcherAssert.assertThat(violations, hasSize(0)); diff --git a/src/lib/services/CommandLineSupport.ts b/src/lib/services/CommandLineSupport.ts index 4ad203958..b5842bba3 100644 --- a/src/lib/services/CommandLineSupport.ts +++ b/src/lib/services/CommandLineSupport.ts @@ -28,7 +28,7 @@ export abstract class CommandLineSupport extends AsyncCreatable { private parentLogger: Logger; private parentInitialized: boolean; - private outputProcessor: OutputProcessor; + protected outputProcessor: OutputProcessor; protected async init(): Promise { @@ -77,10 +77,14 @@ export abstract class CommandLineSupport extends AsyncCreatable { // When data is passed back up to us, pop it onto the appropriate string. cp.stdout.on('data', data => { - stdout += data; + if (!this.handleLiveOut(''+data)) { + stdout += data; + } }); cp.stderr.on('data', data => { - stderr += data; + if (!this.handleLiveErr(''+data)) { + stderr += data; + } }); cp.on('exit', code => { @@ -100,4 +104,26 @@ export abstract class CommandLineSupport extends AsyncCreatable { }); }); } + + /** + * Handles output in realtime. + * Individual engines can override this hook to have custom implementation. + * @param data that was received as stdout + * @returns true if data was handled and false if it needs to be handled later + */ + protected handleLiveOut(data: string): boolean { + // By default, we handle all data at the end + return false; + } + + /** + * Handles error messages in realtime. + * Individual engines can override this hook to have custom implementation. + * @param err that was received as on stderr + * @returns true if data was handled and false if it needs to be handled later + */ + protected handleLiveErr(err: string): boolean { + // By default, we handle all the errors at the end + return false; + } } diff --git a/src/lib/services/OutputProcessor.ts b/src/lib/services/OutputProcessor.ts index bde7eba52..a9bf30ddd 100644 --- a/src/lib/services/OutputProcessor.ts +++ b/src/lib/services/OutputProcessor.ts @@ -8,6 +8,11 @@ Messages.importMessagesDirectory(__dirname); const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'EventKeyTemplates'); const genericMessageKey = 'error.external.genericErrorMessage'; +const MESSAGE_START_TAG = 'SFDX-START'; +const MESSAGE_END_TAG = 'SFDX-END'; +const REALTIME_MESSAGE_START_TAG = 'SFCA-REALTIME-START'; +const REALTIME_MESSAGE_END_TAG = 'SFCA-REALTIME-END'; + /** * Helps with processing output from PmdCatalog java module and converting messages into usable events */ @@ -30,28 +35,40 @@ export class OutputProcessor extends AsyncCreatable { this.initialized = true; } + public isRealtimeOutput(out: string): boolean { + return out.startsWith(REALTIME_MESSAGE_START_TAG); + } + + public processOutput(out: string): boolean { + return this.processAllOutput(out, MESSAGE_START_TAG, MESSAGE_END_TAG); + } + + public processRealtimeOutput(out: string): boolean { + return this.processAllOutput(out, REALTIME_MESSAGE_START_TAG, REALTIME_MESSAGE_END_TAG); + } + // We want to find any events that were dumped into stdout or stderr and turn them back into events that can be thrown. // As per the convention outlined in SfdxMessager.java, SFDX-relevant messages will be stored in the outputs as JSONs - // sandwiched between 'SFDX-START' and 'SFDX-END'. So we'll find all instances of that. - public processOutput(out: string): void { + // sandwiched between a given start tag and end tag. So we'll find all instances of that. + private processAllOutput(out: string, startTag: string, endTag: string): boolean { this.logger.trace(`stdout: ${out}`); if (!out) { // Nothing to do here - return; + return false; } - const outEvents: RuleEvent[] = this.getEventsFromString(out); + const outEvents: RuleEvent[] = this.getEventsFromString(out, startTag, endTag); this.logger.trace(`Total count of events found: ${outEvents.length}`); - this.emitEvents(outEvents); + return this.emitEvents(outEvents); } // TODO: consider moving all message creation logic to a separate place and making this method private - public emitEvents(outEvents: RuleEvent[]): void { + public emitEvents(outEvents: RuleEvent[]): boolean { this.logger.trace('About to order and emit'); // If list is empty, we can just be done now. if (outEvents.length == 0) { - return; + return false; } // Iterate over all of the events and throw them as appropriate. @@ -62,6 +79,7 @@ export class OutputProcessor extends AsyncCreatable { this.emitUxEvent(eventType, event.messageKey, event.args); } }); + return true; } @@ -79,12 +97,13 @@ export class OutputProcessor extends AsyncCreatable { uxEvents.emit(eventType, constructedMessage); } - private getEventsFromString(str: string): RuleEvent[] { + private getEventsFromString(str: string, startTag: string, endTag: string): RuleEvent[] { const events: RuleEvent[] = []; - const regex = /SFDX-START(.*)SFDX-END/g; - const headerLength = 'SFDX-START'.length; - const tailLength = 'SFDX-END'.length; + // const regex = /SFDX-START(.*)SFDX-END/g; + const regex = new RegExp(`^${startTag}(.*)${endTag}`, 'g'); + const headerLength = startTag.length; + const tailLength = endTag.length; const regexMatch = str.match(regex); if (!regexMatch || regexMatch.length < 1) { this.logger.trace(`No events to log`); diff --git a/src/lib/sfge/SfgeWrapper.ts b/src/lib/sfge/SfgeWrapper.ts index 723516852..16e4cf700 100644 --- a/src/lib/sfge/SfgeWrapper.ts +++ b/src/lib/sfge/SfgeWrapper.ts @@ -230,4 +230,11 @@ export class SfgeWrapper extends CommandLineSupport { }); return wrapper.execute(); } + + protected handleLiveOut(data: string): boolean { + if (this.outputProcessor.isRealtimeOutput(data)) { + return this.outputProcessor.processRealtimeOutput(data); + } + return false; + } } From 2def31f1b95b8e37ee5443b293d67d77eac875d3 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Wed, 10 Aug 2022 15:58:38 -0700 Subject: [PATCH 02/13] Added more progress information and stats --- .../com/salesforce/messaging/EventKey.java | 9 +- messages/EventKeyTemplates.js | 11 ++- .../com/salesforce/CliMessagerAppender.java | 32 ++++--- sfge/src/main/java/com/salesforce/Main.java | 13 +-- .../java/com/salesforce/config/EnvUtil.java | 12 +++ .../com/salesforce/config/SfgeConfig.java | 6 ++ .../com/salesforce/config/SfgeConfigImpl.java | 5 ++ .../com/salesforce/graph/ops/GraphUtil.java | 15 ++++ .../metainfo/VisualForceHandlerImpl.java | 2 +- .../salesforce/rules/AbstractRuleRunner.java | 4 + .../salesforce/rules/PathBasedRuleRunner.java | 14 ++- .../rules/ThreadableRuleExecutor.java | 9 +- .../rules/ops/ProgressListener.java | 19 +++- .../rules/ops/ProgressListenerImpl.java | 87 +++++++++++++++++-- .../rules/ops/ProgressListenerProvider.java | 16 ++++ .../test/java/com/salesforce/TestUtil.java | 4 +- .../rules/PathBasedRuleRunnerTest.java | 4 +- 17 files changed, 209 insertions(+), 53 deletions(-) create mode 100644 sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java diff --git a/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java b/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java index adfee16e1..18c0e722b 100644 --- a/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java +++ b/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java @@ -29,9 +29,12 @@ public enum EventKey { /** SFGE RELATED **/ INFO_GENERAL("info.sfgeInfoLog", 1, MessageType.INFO, MessageHandler.UX, true), INFO_META_INFO_COLLECTED("info.sfgeMetaInfoCollected", 2, MessageType.INFO, MessageHandler.UX, true), - INFO_BEGIN_PATH_CREATION("info.sfgeBeginPathCreation", 0, MessageType.INFO, MessageHandler.UX, true), - INFO_END_PATH_CREATION("info.sfgeEndPathCreation", 1, MessageType.INFO, MessageHandler.UX, false), - INFO_PATH_ANALYSIS_PROGRESS("info.sfgePathAnalysisProgress", 2, MessageType.INFO, MessageHandler.UX, true), + INFO_COMPLETED_FILE_COMPILATION("info.sfgeFinishedCompilingFiles", 1, MessageType.INFO, MessageHandler.UX, true), + INFO_STARTED_BUILDING_GRAPH("info.sfgeStartedBuildingGraph", 0, MessageType.INFO, MessageHandler.UX, true), + INFO_COMPLETED_BUILDING_GRAPH("info.sfgeFinishedBuildingGraph", 0, MessageType.INFO, MessageHandler.UX, true), + INFO_PATH_ENTRY_POINTS_IDENTIFIED("info.sfgePathEntryPointsIdentified", 1, MessageType.INFO, MessageHandler.UX, true), + INFO_PATH_ANALYSIS_PROGRESS("info.sfgeViolationsInPathProgress", 3, MessageType.INFO, MessageHandler.UX, true), + INFO_COMPLETED_PATH_ANALYSIS("info.sfgeCompletedPathAnalysis", 3, MessageType.INFO, MessageHandler.UX, false), WARNING_GENERAL("warning.sfgeWarnLog", 1, MessageType.WARNING, MessageHandler.UX, true), WARNING_MULTIPLE_METHOD_TARGET_MATCHES("warning.multipleMethodTargetMatches", 3, MessageType.WARNING, MessageHandler.UX, false), WARNING_NO_METHOD_TARGET_MATCHES("warning.noMethodTargetMatches", 2, MessageType.WARNING, MessageHandler.UX, false), diff --git a/messages/EventKeyTemplates.js b/messages/EventKeyTemplates.js index 3011303a4..80300d7a0 100644 --- a/messages/EventKeyTemplates.js +++ b/messages/EventKeyTemplates.js @@ -9,10 +9,13 @@ module.exports = { "pmdRuleSkipped": "Omitting results for PMD rule \"%s\". Reason: %s.", "unmatchedPathExtensionCpd": "Path extensions for the following files will not be processed by CPD: %s", "sfgeInfoLog": "%s", - "sfgeMetaInfoCollected": "Loaded %s %s", - "sfgeBeginPathCreation": "Building paths across source code.", - "sfgeEndPathCreation": "Completed building paths across source code. Count of paths detected: %s", - "sfgePathAnalysisProgress": "Analyzed %s of %s paths." + "sfgeMetaInfoCollected": "Loaded %s: [ %s ]", + "sfgeFinishedCompilingFiles": "Compiled %s files.", + "sfgeStartedBuildingGraph": "Building graph . . .", + "sfgeFinishedBuildingGraph": "Added all compilation units to graph.", + "sfgePathEntryPointsIdentified": "Identified %s path entry point(s).", + "sfgeViolationsInPathProgress": "Detected %s violation(s) from %s path(s) on %s entry point(s) so far.", + "sfgeCompletedPathAnalysis": "Overall, analyzed %s path(s) from %s entry point(s). Detected %s violation(s)." }, "warning": { "invalidCategorySkipped": "Cataloger skipped invalid PMD Category file '%s'.", diff --git a/sfge/src/main/java/com/salesforce/CliMessagerAppender.java b/sfge/src/main/java/com/salesforce/CliMessagerAppender.java index d8cbf1c1f..f5a84d691 100644 --- a/sfge/src/main/java/com/salesforce/CliMessagerAppender.java +++ b/sfge/src/main/java/com/salesforce/CliMessagerAppender.java @@ -1,5 +1,6 @@ package com.salesforce; +import com.salesforce.config.SfgeConfigProvider; import com.salesforce.messaging.CliMessager; import com.salesforce.messaging.EventKey; import java.io.Serializable; @@ -26,14 +27,16 @@ elementType = Appender.ELEMENT_TYPE) public class CliMessagerAppender extends AbstractAppender { + private final boolean shouldLogWarningsOnVerbose; + @PluginFactory public static CliMessagerAppender createAppender( @PluginAttribute("name") String name, @PluginElement("Layout") Layout layout, @PluginElement("Filter") final Filter filter) { if (name == null) { - LOGGER.error("There is no name provided for MyCustomAppender"); - return null; + // Assign default name to avoid complaining + name = "CliMessagerAppender"; } if (layout == null) { layout = PatternLayout.createDefaultLayout(); @@ -47,23 +50,32 @@ protected CliMessagerAppender( Layout layout, final boolean ignoreExceptions) { super(name, filter, layout, ignoreExceptions, null); + this.shouldLogWarningsOnVerbose = SfgeConfigProvider.get().shouldLogWarningsOnVerbose(); } + /** + * {@link CliMessagerAppender} decrements the log level while publishing to CLI. Warning is + * reduced to Info, Error is reduced to Warning, Fatal is reduced to Error. + * + * @param event that was published from code + */ @Override public void append(LogEvent event) { Level level = event.getLevel(); - if (Level.INFO.equals(level)) { - CliMessager.postMessage("SFGE Info log", EventKey.INFO_GENERAL, getEventMessage(event)); - } else if (Level.WARN.equals(level)) { - CliMessager.postMessage("SFGE Warn", EventKey.WARNING_GENERAL, getEventMessage(event)); - } else if (Level.ERROR.equals(level) || Level.FATAL.equals(level)) { + if (Level.WARN.equals(level) && this.shouldLogWarningsOnVerbose) { + CliMessager.postMessage( + "SFGE Warn as Info", EventKey.INFO_GENERAL, getEventMessage(event)); + } else if (Level.ERROR.equals(level)) { + CliMessager.postMessage( + "SFGE Error as Warning", EventKey.WARNING_GENERAL, getEventMessage(event)); + } else if (Level.FATAL.equals(level)) { CliMessager.postMessage( - "SFGE Error/Fatal", EventKey.ERROR_GENERAL, getEventMessage(event)); + "SFGE Fatal as Error", EventKey.ERROR_GENERAL, getEventMessage(event)); } else { - // TODO: revisit + // TODO: revisit how the outliers are handled error( String.format( - "Unable to log less than INFO level [{}]: {}", + "Unable to log less than WARN level [{}]: {}", event.getLevel(), getEventMessage(event))); } diff --git a/sfge/src/main/java/com/salesforce/Main.java b/sfge/src/main/java/com/salesforce/Main.java index 539160146..4634f88d7 100644 --- a/sfge/src/main/java/com/salesforce/Main.java +++ b/sfge/src/main/java/com/salesforce/Main.java @@ -8,13 +8,13 @@ import com.salesforce.exception.UserActionException; import com.salesforce.graph.ops.GraphUtil; import com.salesforce.messaging.CliMessager; -import com.salesforce.messaging.EventKey; import com.salesforce.metainfo.MetaInfoCollector; import com.salesforce.metainfo.MetaInfoCollectorProvider; import com.salesforce.rules.AbstractRule; import com.salesforce.rules.RuleRunner; import com.salesforce.rules.RuleUtil; import com.salesforce.rules.Violation; +import com.salesforce.rules.ops.ProgressListenerProvider; import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -147,6 +147,7 @@ private int execute(String... args) { List allViolations; try { allViolations = new RuleRunner(g).runRules(eap.getSelectedRules(), eap.getTargets()); + ProgressListenerProvider.get().completedAnalysis(); } catch (SfgeRuntimeException ex) { LOGGER.error("Error while running rules", ex); System.err.println(formatError(ex)); @@ -173,11 +174,11 @@ private void collectMetaInfo(CliArgParser.ExecuteArgParser eap) { for (MetaInfoCollector collector : allCollectors) { collector.loadProjectFiles(eap.getProjectDirs()); - CliMessager.postMessage( - "Meta information collected", - EventKey.INFO_META_INFO_COLLECTED, - String.valueOf(collector.getMetaInfoCollected().size()), - collector.getMetaInfoTypeName()); + + // Let progress listener know about the meta information collected + ProgressListenerProvider.get() + .collectedMetaInfo( + collector.getMetaInfoTypeName(), collector.getMetaInfoCollected()); } } diff --git a/sfge/src/main/java/com/salesforce/config/EnvUtil.java b/sfge/src/main/java/com/salesforce/config/EnvUtil.java index b2b913675..4418d2dbd 100644 --- a/sfge/src/main/java/com/salesforce/config/EnvUtil.java +++ b/sfge/src/main/java/com/salesforce/config/EnvUtil.java @@ -9,6 +9,7 @@ public final class EnvUtil { private static final String ENV_RULE_ENABLE_WARNING_VIOLATION = "SFGE_RULE_ENABLE_WARNING_VIOLATION"; private static final String ENV_IGNORE_PARSE_ERRORS = "SFGE_IGNORE_PARSE_ERRORS"; + private static final String ENV_LOG_WARNINGS_ON_VERBOSE = "SFGE_LOG_WARNINGS_ON_VERBOSE"; // TODO: These should move to SfgeConfigImpl and this class should return Optionals @VisibleForTesting @@ -21,6 +22,7 @@ public final class EnvUtil { @VisibleForTesting static final boolean DEFAULT_RULE_ENABLE_WARNING_VIOLATION = true; @VisibleForTesting static final boolean DEFAULT_IGNORE_PARSE_ERRORS = false; + @VisibleForTesting static final boolean DEFAULT_LOG_WARNINGS_ON_VERBOSE = false; /** * Returns the value of the {@link #ENV_RULE_THREAD_COUNT} environment variable if set, else @@ -61,6 +63,16 @@ static boolean shouldIgnoreParseErrors() { return getBoolOrDefault(ENV_IGNORE_PARSE_ERRORS, DEFAULT_IGNORE_PARSE_ERRORS); } + /** + * Indicates if SFGE should log internal warnings on --verbose + * + * @return value of {@link #ENV_LOG_WARNINGS_ON_VERBOSE} env variable. If it is not set, {@link + * #DEFAULT_LOG_WARNINGS_ON_VERBOSE} + */ + static boolean shouldLogWarningsOnVerbose() { + return getBoolOrDefault(ENV_LOG_WARNINGS_ON_VERBOSE, DEFAULT_LOG_WARNINGS_ON_VERBOSE); + } + private static int getIntOrDefault(String name, int defaultValue) { String strVal = System.getProperty(name); return strVal == null ? defaultValue : Integer.parseInt(strVal); diff --git a/sfge/src/main/java/com/salesforce/config/SfgeConfig.java b/sfge/src/main/java/com/salesforce/config/SfgeConfig.java index 1503614e9..e10b00383 100644 --- a/sfge/src/main/java/com/salesforce/config/SfgeConfig.java +++ b/sfge/src/main/java/com/salesforce/config/SfgeConfig.java @@ -16,4 +16,10 @@ public interface SfgeConfig { /** Indicates if a Jorje parse error causes the entire process to stop. */ boolean shouldIgnoreParseErrors(); + + /** + * Indicates if Warn level logs to log4j should be forwarded to CLI as well when verbose is + * enabled + */ + boolean shouldLogWarningsOnVerbose(); } diff --git a/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java b/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java index fd45a735e..13d081407 100644 --- a/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java +++ b/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java @@ -25,6 +25,11 @@ public boolean shouldIgnoreParseErrors() { return EnvUtil.shouldIgnoreParseErrors(); } + @Override + public boolean shouldLogWarningsOnVerbose() { + return EnvUtil.shouldLogWarningsOnVerbose(); + } + static SfgeConfigImpl getInstance() { return SfgeConfigImpl.LazyHolder.INSTANCE; } diff --git a/sfge/src/main/java/com/salesforce/graph/ops/GraphUtil.java b/sfge/src/main/java/com/salesforce/graph/ops/GraphUtil.java index b6385ddfe..c414eb199 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/GraphUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/GraphUtil.java @@ -20,6 +20,8 @@ import com.salesforce.graph.vertex.BaseSFVertex; import com.salesforce.graph.vertex.SFVertexFactory; import com.salesforce.io.FileHandler; +import com.salesforce.rules.ops.ProgressListener; +import com.salesforce.rules.ops.ProgressListenerProvider; import java.io.File; import java.io.IOException; import java.nio.file.FileVisitResult; @@ -154,8 +156,17 @@ public static void loadSourceFolders(GraphTraversalSource g, List source } } + final ProgressListener progressListener = ProgressListenerProvider.get(); + + // Let progress listener know that we've finished compiling all the files in project + progressListener.finishedFileCompilation(); + Util.Config config = Util.Config.Builder.get(g, comps).build(); + + // Let progress listener know what we are doing + progressListener.startedBuildingGraph(); Util.buildGraph(config); + progressListener.completedBuildingGraph(); } private static List buildFolderComps(String sourceFolder) @@ -182,6 +193,8 @@ private static List buildFolderComps(String sourceFo private static Optional loadFile(Path path) throws IOException { String pathString = path.toString(); + final ProgressListener progressListener = ProgressListenerProvider.get(); + if (!pathString.toLowerCase(Locale.ROOT).endsWith(".cls")) { if (LOGGER.isDebugEnabled()) { LOGGER.debug("Skipping file. path=" + pathString); @@ -194,6 +207,8 @@ private static Optional loadFile(Path path) throws I } String sourceCode = FileHandler.getInstance().readTargetFile(pathString); AstNodeWrapper astNodeWrapper = JorjeUtil.compileApexFromString(sourceCode); + // Let progress listener know that we've compiled another file + progressListener.compiledAnotherFile(); return Optional.of(new Util.CompilationDescriptor(pathString, astNodeWrapper)); } catch (JorjeUtil.JorjeCompilationException ex) { if (SfgeConfigProvider.get().shouldIgnoreParseErrors()) { diff --git a/sfge/src/main/java/com/salesforce/metainfo/VisualForceHandlerImpl.java b/sfge/src/main/java/com/salesforce/metainfo/VisualForceHandlerImpl.java index fec93870a..85c6b5e14 100644 --- a/sfge/src/main/java/com/salesforce/metainfo/VisualForceHandlerImpl.java +++ b/sfge/src/main/java/com/salesforce/metainfo/VisualForceHandlerImpl.java @@ -15,7 +15,7 @@ public class VisualForceHandlerImpl extends AbstractMetaInfoCollector { private static final Logger LOGGER = LogManager.getLogger(VisualForceHandlerImpl.class); - private static final String META_INFO_TYPE_NAME = "Apex Controller entry points"; + private static final String META_INFO_TYPE_NAME = "Apex Controllers"; @Override protected TreeSet getAcceptedExtensions() { diff --git a/sfge/src/main/java/com/salesforce/rules/AbstractRuleRunner.java b/sfge/src/main/java/com/salesforce/rules/AbstractRuleRunner.java index b97595a82..f1b773db9 100644 --- a/sfge/src/main/java/com/salesforce/rules/AbstractRuleRunner.java +++ b/sfge/src/main/java/com/salesforce/rules/AbstractRuleRunner.java @@ -6,6 +6,7 @@ import com.salesforce.graph.Schema; import com.salesforce.graph.build.CaseSafePropertyUtil.H; import com.salesforce.graph.vertex.MethodVertex; +import com.salesforce.rules.ops.ProgressListenerProvider; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -87,6 +88,9 @@ private Set runPathBasedRules( return new HashSet<>(); } + // Let listener know that we have finished identifying entry points in target + ProgressListenerProvider.get().pathEntryPointsIdentified(pathEntryPoints.size()); + // For each entry point, generate a submission object. List submissions = new ArrayList<>(); for (MethodVertex pathEntryPoint : pathEntryPoints) { diff --git a/sfge/src/main/java/com/salesforce/rules/PathBasedRuleRunner.java b/sfge/src/main/java/com/salesforce/rules/PathBasedRuleRunner.java index 63626c6e4..0aa527108 100644 --- a/sfge/src/main/java/com/salesforce/rules/PathBasedRuleRunner.java +++ b/sfge/src/main/java/com/salesforce/rules/PathBasedRuleRunner.java @@ -11,6 +11,7 @@ import com.salesforce.rules.fls.apex.operations.FlsViolationInfo; import com.salesforce.rules.fls.apex.operations.FlsViolationMessageUtil; import com.salesforce.rules.ops.ProgressListener; +import com.salesforce.rules.ops.ProgressListenerProvider; import java.util.HashSet; import java.util.List; import java.util.Optional; @@ -32,15 +33,12 @@ public class PathBasedRuleRunner { private final ProgressListener progressListener; public PathBasedRuleRunner( - GraphTraversalSource g, - List rules, - MethodVertex methodVertex, - ProgressListener progressListener) { + GraphTraversalSource g, List rules, MethodVertex methodVertex) { this.g = g; this.rules = rules; this.methodVertex = methodVertex; this.violations = new HashSet<>(); - this.progressListener = progressListener; + this.progressListener = ProgressListenerProvider.get(); } /** @@ -52,10 +50,8 @@ public Set runRules() { // Build configuration to define how apex paths will be expanded final ApexPathExpanderConfig expanderConfig = getApexPathExpanderConfig(); - progressListener.initializingPathCreation(); // Get all the paths that originate in the entry point final List paths = getPaths(expanderConfig); - progressListener.identifiedPaths(paths); // Execute rules on the paths found executeRulesOnPaths(paths); @@ -74,6 +70,8 @@ public Set runRules() { } } + progressListener.finishedAnalyzingEntryPoint(paths, violations); + return violations; } @@ -90,8 +88,6 @@ private void executeRulesOnPaths(List paths) { // Iterate over all vertices in the path... for (ApexPathVertexMetaInfo.PredicateMatch predicateMatch : path.getApexPathMetaInfo().get().getAllMatches()) { - progressListener.pickedNewPathForAnalysis(path); - AbstractPathBasedRule rule = (AbstractPathBasedRule) predicateMatch.getPredicate(); BaseSFVertex vertex = predicateMatch.getPathVertex().getVertex(); diff --git a/sfge/src/main/java/com/salesforce/rules/ThreadableRuleExecutor.java b/sfge/src/main/java/com/salesforce/rules/ThreadableRuleExecutor.java index ef4aa99e3..72724e1d5 100644 --- a/sfge/src/main/java/com/salesforce/rules/ThreadableRuleExecutor.java +++ b/sfge/src/main/java/com/salesforce/rules/ThreadableRuleExecutor.java @@ -6,7 +6,7 @@ import com.salesforce.graph.ops.LogUtil; import com.salesforce.graph.vertex.MethodVertex; import com.salesforce.rules.ops.ProgressListener; -import com.salesforce.rules.ops.ProgressListenerImpl; +import com.salesforce.rules.ops.ProgressListenerProvider; import java.util.List; import java.util.Set; import java.util.Timer; @@ -80,7 +80,7 @@ private static int submitRunners( CompletionService> completionService, List submissions) { int submissionCount = 0; - final ProgressListener progressListener = new ProgressListenerImpl(); + final ProgressListener progressListener = ProgressListenerProvider.get(); for (ThreadableRuleSubmission submission : submissions) { completionService.submit(new CallableExecutor(submission, progressListener)); submissionCount += 1; @@ -114,13 +114,11 @@ private static Set waitForCallable( private static class CallableExecutor implements Callable> { private final ThreadableRuleSubmission submission; - private final ProgressListener progressListener; private boolean timedOut; public CallableExecutor( ThreadableRuleSubmission submission, ProgressListener progressListener) { this.submission = submission; - this.progressListener = progressListener; } public Set call() { @@ -157,8 +155,7 @@ public void run() { new PathBasedRuleRunner( submission.getGraph(), submission.getRules(), - submission.getPathEntry(), - progressListener) + submission.getPathEntry()) .runRules()); timer.cancel(); } catch (StackOverflowError | Exception ex) { diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java index 8763ad9cc..7d56975a6 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java @@ -1,12 +1,25 @@ package com.salesforce.rules.ops; import com.salesforce.graph.ApexPath; +import com.salesforce.rules.Violation; import java.util.List; +import java.util.Set; +import java.util.TreeSet; public interface ProgressListener { - void initializingPathCreation(); + void collectedMetaInfo(String metaInfoType, TreeSet itemsCollected); - void identifiedPaths(List paths); + void compiledAnotherFile(); - void pickedNewPathForAnalysis(ApexPath path); + void finishedFileCompilation(); + + void startedBuildingGraph(); + + void completedBuildingGraph(); + + void pathEntryPointsIdentified(int pathEntryPointsCount); + + void finishedAnalyzingEntryPoint(List paths, Set violations); + + void completedAnalysis(); } diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java index b76cdc30d..c64c61930 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java @@ -1,22 +1,99 @@ package com.salesforce.rules.ops; +import com.google.common.base.Joiner; import com.salesforce.graph.ApexPath; import com.salesforce.messaging.CliMessager; import com.salesforce.messaging.EventKey; +import com.salesforce.rules.Violation; import java.util.List; +import java.util.Set; +import java.util.TreeSet; +/** Publishes information to CLI on the progress of analysis. */ public class ProgressListenerImpl implements ProgressListener { + private int filesCompiled = 0; + private int pathsDetected = 0; + private int violationsDetected = 0; + private int entryPointsAnalyzed = 0; + + private static final int PROGRESS_INCREMENTS = 10; // TODO: get this value from config + + static ProgressListener getInstance() { + return ProgressListenerImpl.LazyHolder.INSTANCE; + } + + private static final class LazyHolder { + private static final ProgressListenerImpl INSTANCE = new ProgressListenerImpl(); + } + + private ProgressListenerImpl() {} + + @Override + public void collectedMetaInfo(String metaInfoType, TreeSet itemsCollected) { + final String items = + (itemsCollected.isEmpty()) ? "none found" : Joiner.on(',').join(itemsCollected); + CliMessager.postMessage( + "Meta information collected", + EventKey.INFO_META_INFO_COLLECTED, + metaInfoType, + items); + } + + @Override + public void compiledAnotherFile() { + filesCompiled++; + } + + @Override + public void finishedFileCompilation() { + CliMessager.postMessage( + "Finished compiling files", + EventKey.INFO_COMPLETED_FILE_COMPILATION, + String.valueOf(filesCompiled)); + } + + @Override + public void startedBuildingGraph() { + CliMessager.postMessage("Started building graph", EventKey.INFO_STARTED_BUILDING_GRAPH); + } + + @Override + public void completedBuildingGraph() { + CliMessager.postMessage("Finished building graph", EventKey.INFO_COMPLETED_BUILDING_GRAPH); + } + @Override - public void initializingPathCreation() { - CliMessager.postMessage("", EventKey.INFO_BEGIN_PATH_CREATION); + public void pathEntryPointsIdentified(int pathEntryPointsCount) { + CliMessager.postMessage( + "Path entry points identified", + EventKey.INFO_PATH_ENTRY_POINTS_IDENTIFIED, + String.valueOf(pathEntryPointsCount)); } @Override - public void identifiedPaths(List paths) { - CliMessager.postMessage("", EventKey.INFO_END_PATH_CREATION, String.valueOf(paths.size())); + public void finishedAnalyzingEntryPoint(List paths, Set violations) { + pathsDetected += paths.size(); + violationsDetected += violations.size(); + entryPointsAnalyzed++; + + if (pathsDetected % PROGRESS_INCREMENTS == 0) { + CliMessager.postMessage( + "Count of violations in paths, entry points", + EventKey.INFO_PATH_ANALYSIS_PROGRESS, + String.valueOf(violationsDetected), + String.valueOf(pathsDetected), + String.valueOf(entryPointsAnalyzed)); + } } @Override - public void pickedNewPathForAnalysis(ApexPath path) {} + public void completedAnalysis() { + CliMessager.postMessage( + "Completed analysis stats", + EventKey.INFO_COMPLETED_PATH_ANALYSIS, + String.valueOf(pathsDetected), + String.valueOf(entryPointsAnalyzed), + String.valueOf(violationsDetected)); + } } diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java new file mode 100644 index 000000000..379da954d --- /dev/null +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java @@ -0,0 +1,16 @@ +package com.salesforce.rules.ops; + +import com.google.common.annotations.VisibleForTesting; + +public class ProgressListenerProvider { + @VisibleForTesting + static final ThreadLocal PROGRESS_LISTENER = + ThreadLocal.withInitial(() -> ProgressListenerImpl.getInstance()); + + /** Get the ProgressListener for the current thread */ + public static ProgressListener get() { + return PROGRESS_LISTENER.get(); + } + + private ProgressListenerProvider() {} +} diff --git a/sfge/src/test/java/com/salesforce/TestUtil.java b/sfge/src/test/java/com/salesforce/TestUtil.java index 683fd1749..a14d208ed 100644 --- a/sfge/src/test/java/com/salesforce/TestUtil.java +++ b/sfge/src/test/java/com/salesforce/TestUtil.java @@ -38,7 +38,6 @@ import com.salesforce.rules.PathBasedRuleRunner; import com.salesforce.rules.StaticRule; import com.salesforce.rules.Violation; -import com.salesforce.rules.ops.ProgressListenerImpl; import java.io.File; import java.io.IOException; import java.lang.reflect.Method; @@ -312,8 +311,7 @@ public static List getViolations( final MethodVertex methodVertex = getMethodVertex(g, definingType, methodName); final PathBasedRuleRunner ruleRunner = - new PathBasedRuleRunner( - g, Lists.newArrayList(rule), methodVertex, new ProgressListenerImpl()); + new PathBasedRuleRunner(g, Lists.newArrayList(rule), methodVertex); final List violations = new ArrayList<>(ruleRunner.runRules()); return violations; diff --git a/sfge/src/test/java/com/salesforce/rules/PathBasedRuleRunnerTest.java b/sfge/src/test/java/com/salesforce/rules/PathBasedRuleRunnerTest.java index 8aa6abef8..c05c3cf17 100644 --- a/sfge/src/test/java/com/salesforce/rules/PathBasedRuleRunnerTest.java +++ b/sfge/src/test/java/com/salesforce/rules/PathBasedRuleRunnerTest.java @@ -4,7 +4,6 @@ import com.salesforce.TestUtil; import com.salesforce.graph.vertex.MethodVertex; -import com.salesforce.rules.ops.ProgressListenerImpl; import java.util.Collections; import java.util.List; import java.util.Set; @@ -43,8 +42,7 @@ public void pathsWithoutInterestingVerticesAreIgnored() { Collections.singletonList(ApexFlsViolationRule.getInstance()); // Define a PathBasedRuleRunner to apply the rule against the method vertex. - PathBasedRuleRunner runner = - new PathBasedRuleRunner(g, rules, methodVertex, new ProgressListenerImpl()); + PathBasedRuleRunner runner = new PathBasedRuleRunner(g, rules, methodVertex); Set violations = runner.runRules(); MatcherAssert.assertThat(violations, hasSize(0)); From 56fed64ff3e734d50a86cb7e7275e1b862a6520c Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Wed, 10 Aug 2022 16:27:21 -0700 Subject: [PATCH 03/13] Config related changes --- .../java/com/salesforce/CliMessagerAppender.java | 3 +-- .../main/java/com/salesforce/config/EnvUtil.java | 15 +++++++++++++++ .../java/com/salesforce/config/SfgeConfig.java | 6 ++++++ .../com/salesforce/config/SfgeConfigImpl.java | 5 +++++ .../rules/ops/ProgressListenerImpl.java | 9 ++++++--- .../config/SfgeConfigProviderTest.java | 16 ++++++++++++++++ .../com/salesforce/config/TestSfgeConfig.java | 10 ++++++++++ 7 files changed, 59 insertions(+), 5 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/CliMessagerAppender.java b/sfge/src/main/java/com/salesforce/CliMessagerAppender.java index f5a84d691..9fe793a34 100644 --- a/sfge/src/main/java/com/salesforce/CliMessagerAppender.java +++ b/sfge/src/main/java/com/salesforce/CliMessagerAppender.java @@ -64,7 +64,7 @@ public void append(LogEvent event) { Level level = event.getLevel(); if (Level.WARN.equals(level) && this.shouldLogWarningsOnVerbose) { CliMessager.postMessage( - "SFGE Warn as Info", EventKey.INFO_GENERAL, getEventMessage(event)); + "SFGE Warning as Info", EventKey.INFO_GENERAL, getEventMessage(event)); } else if (Level.ERROR.equals(level)) { CliMessager.postMessage( "SFGE Error as Warning", EventKey.WARNING_GENERAL, getEventMessage(event)); @@ -72,7 +72,6 @@ public void append(LogEvent event) { CliMessager.postMessage( "SFGE Fatal as Error", EventKey.ERROR_GENERAL, getEventMessage(event)); } else { - // TODO: revisit how the outliers are handled error( String.format( "Unable to log less than WARN level [{}]: {}", diff --git a/sfge/src/main/java/com/salesforce/config/EnvUtil.java b/sfge/src/main/java/com/salesforce/config/EnvUtil.java index 4418d2dbd..7ff3a07db 100644 --- a/sfge/src/main/java/com/salesforce/config/EnvUtil.java +++ b/sfge/src/main/java/com/salesforce/config/EnvUtil.java @@ -10,6 +10,8 @@ public final class EnvUtil { "SFGE_RULE_ENABLE_WARNING_VIOLATION"; private static final String ENV_IGNORE_PARSE_ERRORS = "SFGE_IGNORE_PARSE_ERRORS"; private static final String ENV_LOG_WARNINGS_ON_VERBOSE = "SFGE_LOG_WARNINGS_ON_VERBOSE"; + private static final String ENV_PROGRESS_INCREMENTS_ON_VERBOSE = + "SFGE_PROGRESS_INCREMENTS_ON_VERBOSE"; // TODO: These should move to SfgeConfigImpl and this class should return Optionals @VisibleForTesting @@ -23,6 +25,7 @@ public final class EnvUtil { @VisibleForTesting static final boolean DEFAULT_RULE_ENABLE_WARNING_VIOLATION = true; @VisibleForTesting static final boolean DEFAULT_IGNORE_PARSE_ERRORS = false; @VisibleForTesting static final boolean DEFAULT_LOG_WARNINGS_ON_VERBOSE = false; + @VisibleForTesting static final int DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE = 50; /** * Returns the value of the {@link #ENV_RULE_THREAD_COUNT} environment variable if set, else @@ -73,6 +76,18 @@ static boolean shouldLogWarningsOnVerbose() { return getBoolOrDefault(ENV_LOG_WARNINGS_ON_VERBOSE, DEFAULT_LOG_WARNINGS_ON_VERBOSE); } + /** + * Gets the level of increments at which path analysis progress update is provided. Applicable + * only with --verbose. + * + * @return value of {@link #ENV_PROGRESS_INCREMENTS_ON_VERBOSE} environment variable if set, + * else {@link #DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE} + */ + static int getProgressIncrementsOnVerbose() { + return getIntOrDefault( + ENV_PROGRESS_INCREMENTS_ON_VERBOSE, DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE); + } + private static int getIntOrDefault(String name, int defaultValue) { String strVal = System.getProperty(name); return strVal == null ? defaultValue : Integer.parseInt(strVal); diff --git a/sfge/src/main/java/com/salesforce/config/SfgeConfig.java b/sfge/src/main/java/com/salesforce/config/SfgeConfig.java index e10b00383..6e3d1e4d4 100644 --- a/sfge/src/main/java/com/salesforce/config/SfgeConfig.java +++ b/sfge/src/main/java/com/salesforce/config/SfgeConfig.java @@ -22,4 +22,10 @@ public interface SfgeConfig { * enabled */ boolean shouldLogWarningsOnVerbose(); + + /** + * Should be used to set the level of increments at which path analysis progress update is + * provided + */ + int getProgressIncrementsOnVerbose(); } diff --git a/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java b/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java index 13d081407..89f39c53a 100644 --- a/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java +++ b/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java @@ -30,6 +30,11 @@ public boolean shouldLogWarningsOnVerbose() { return EnvUtil.shouldLogWarningsOnVerbose(); } + @Override + public int getProgressIncrementsOnVerbose() { + return EnvUtil.getProgressIncrementsOnVerbose(); + } + static SfgeConfigImpl getInstance() { return SfgeConfigImpl.LazyHolder.INSTANCE; } diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java index c64c61930..4d479cfcf 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java @@ -1,6 +1,7 @@ package com.salesforce.rules.ops; import com.google.common.base.Joiner; +import com.salesforce.config.SfgeConfigProvider; import com.salesforce.graph.ApexPath; import com.salesforce.messaging.CliMessager; import com.salesforce.messaging.EventKey; @@ -17,7 +18,7 @@ public class ProgressListenerImpl implements ProgressListener { private int violationsDetected = 0; private int entryPointsAnalyzed = 0; - private static final int PROGRESS_INCREMENTS = 10; // TODO: get this value from config + private final int progressIncrements; static ProgressListener getInstance() { return ProgressListenerImpl.LazyHolder.INSTANCE; @@ -27,7 +28,9 @@ private static final class LazyHolder { private static final ProgressListenerImpl INSTANCE = new ProgressListenerImpl(); } - private ProgressListenerImpl() {} + private ProgressListenerImpl() { + progressIncrements = SfgeConfigProvider.get().getProgressIncrementsOnVerbose(); + } @Override public void collectedMetaInfo(String metaInfoType, TreeSet itemsCollected) { @@ -77,7 +80,7 @@ public void finishedAnalyzingEntryPoint(List paths, Set vio violationsDetected += violations.size(); entryPointsAnalyzed++; - if (pathsDetected % PROGRESS_INCREMENTS == 0) { + if (pathsDetected % progressIncrements == 0) { CliMessager.postMessage( "Count of violations in paths, entry points", EventKey.INFO_PATH_ANALYSIS_PROGRESS, diff --git a/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java b/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java index 0afed7b93..f5378c9c7 100644 --- a/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java +++ b/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java @@ -41,6 +41,16 @@ public boolean isWarningViolationEnabled() { public boolean shouldIgnoreParseErrors() { return !EnvUtil.DEFAULT_IGNORE_PARSE_ERRORS; } + + @Override + public boolean shouldLogWarningsOnVerbose() { + return !EnvUtil.DEFAULT_LOG_WARNINGS_ON_VERBOSE; + } + + @Override + public int getProgressIncrementsOnVerbose() { + return -1 * EnvUtil.DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE; + } }); final SfgeConfig sfgeConfig = SfgeConfigProvider.get(); @@ -57,6 +67,12 @@ public boolean shouldIgnoreParseErrors() { MatcherAssert.assertThat( sfgeConfig.shouldIgnoreParseErrors(), equalTo(!EnvUtil.DEFAULT_IGNORE_PARSE_ERRORS)); + MatcherAssert.assertThat( + sfgeConfig.shouldLogWarningsOnVerbose(), + equalTo(!EnvUtil.DEFAULT_LOG_WARNINGS_ON_VERBOSE)); + MatcherAssert.assertThat( + sfgeConfig.getProgressIncrementsOnVerbose(), + equalTo(-1 * EnvUtil.getProgressIncrementsOnVerbose())); } finally { SfgeConfigTestProvider.remove(); } diff --git a/sfge/src/test/java/com/salesforce/config/TestSfgeConfig.java b/sfge/src/test/java/com/salesforce/config/TestSfgeConfig.java index 13fe0adeb..b705c4527 100644 --- a/sfge/src/test/java/com/salesforce/config/TestSfgeConfig.java +++ b/sfge/src/test/java/com/salesforce/config/TestSfgeConfig.java @@ -24,4 +24,14 @@ public boolean isWarningViolationEnabled() { public boolean shouldIgnoreParseErrors() { return SfgeConfigImpl.getInstance().shouldIgnoreParseErrors(); } + + @Override + public boolean shouldLogWarningsOnVerbose() { + return SfgeConfigImpl.getInstance().shouldLogWarningsOnVerbose(); + } + + @Override + public int getProgressIncrementsOnVerbose() { + return SfgeConfigImpl.getInstance().getProgressIncrementsOnVerbose(); + } } From e97100d23007cf27166d9484128636be78f1525e Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Thu, 11 Aug 2022 14:10:36 -0700 Subject: [PATCH 04/13] Adding more tests + fixing minor issues --- .../rules/ops/ProgressListenerImpl.java | 53 +++++++++++- .../config/SfgeConfigProviderTest.java | 6 ++ .../rules/ops/ProgressListenerImplTest.java | 86 +++++++++++++++++++ src/lib/services/CommandLineSupport.ts | 6 +- src/lib/services/OutputProcessor.ts | 1 - test/commands/scanner/run.dfa.test.ts | 81 +++++++++++++++++ 6 files changed, 227 insertions(+), 6 deletions(-) create mode 100644 sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java create mode 100644 test/commands/scanner/run.dfa.test.ts diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java index 4d479cfcf..3f9989032 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java @@ -1,11 +1,13 @@ package com.salesforce.rules.ops; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.salesforce.config.SfgeConfigProvider; import com.salesforce.graph.ApexPath; import com.salesforce.messaging.CliMessager; import com.salesforce.messaging.EventKey; import com.salesforce.rules.Violation; +import java.util.Collection; import java.util.List; import java.util.Set; import java.util.TreeSet; @@ -13,8 +15,11 @@ /** Publishes information to CLI on the progress of analysis. */ public class ProgressListenerImpl implements ProgressListener { + @VisibleForTesting static final String NONE_FOUND = "none found"; + private int filesCompiled = 0; private int pathsDetected = 0; + private int lastPathCountReported = 0; private int violationsDetected = 0; private int entryPointsAnalyzed = 0; @@ -34,8 +39,7 @@ private ProgressListenerImpl() { @Override public void collectedMetaInfo(String metaInfoType, TreeSet itemsCollected) { - final String items = - (itemsCollected.isEmpty()) ? "none found" : Joiner.on(',').join(itemsCollected); + final String items = stringify(itemsCollected); CliMessager.postMessage( "Meta information collected", EventKey.INFO_META_INFO_COLLECTED, @@ -80,13 +84,17 @@ public void finishedAnalyzingEntryPoint(List paths, Set vio violationsDetected += violations.size(); entryPointsAnalyzed++; - if (pathsDetected % progressIncrements == 0) { + // Make a post only if we have more paths detected than the progress increments + // since the last time we posted. + if (pathsDetected - lastPathCountReported >= progressIncrements) { CliMessager.postMessage( "Count of violations in paths, entry points", EventKey.INFO_PATH_ANALYSIS_PROGRESS, String.valueOf(violationsDetected), String.valueOf(pathsDetected), String.valueOf(entryPointsAnalyzed)); + + lastPathCountReported = pathsDetected; } } @@ -99,4 +107,43 @@ public void completedAnalysis() { String.valueOf(entryPointsAnalyzed), String.valueOf(violationsDetected)); } + + @VisibleForTesting + String stringify(Collection items) { + return (items.isEmpty()) ? NONE_FOUND : Joiner.on(',').join(items); + } + + @VisibleForTesting + void reset() { + filesCompiled = 0; + pathsDetected = 0; + lastPathCountReported = 0; + violationsDetected = 0; + entryPointsAnalyzed = 0; + } + + @VisibleForTesting + int getFilesCompiled() { + return filesCompiled; + } + + @VisibleForTesting + int getPathsDetected() { + return pathsDetected; + } + + @VisibleForTesting + int getLastPathCountReported() { + return lastPathCountReported; + } + + @VisibleForTesting + int getViolationsDetected() { + return violationsDetected; + } + + @VisibleForTesting + int getEntryPointsAnalyzed() { + return entryPointsAnalyzed; + } } diff --git a/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java b/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java index f5378c9c7..8402613a1 100644 --- a/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java +++ b/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java @@ -96,5 +96,11 @@ private void assertDefaultImplementation(SfgeConfig sfgeConfig) { equalTo(EnvUtil.DEFAULT_RULE_ENABLE_WARNING_VIOLATION)); MatcherAssert.assertThat( sfgeConfig.shouldIgnoreParseErrors(), equalTo(EnvUtil.DEFAULT_IGNORE_PARSE_ERRORS)); + MatcherAssert.assertThat( + sfgeConfig.shouldLogWarningsOnVerbose(), + equalTo(EnvUtil.DEFAULT_LOG_WARNINGS_ON_VERBOSE)); + MatcherAssert.assertThat( + sfgeConfig.getProgressIncrementsOnVerbose(), + equalTo(EnvUtil.DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE)); } } diff --git a/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java b/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java new file mode 100644 index 000000000..c7a10b625 --- /dev/null +++ b/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java @@ -0,0 +1,86 @@ +package com.salesforce.rules.ops; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +import com.salesforce.config.SfgeConfigTestProvider; +import com.salesforce.config.TestSfgeConfig; +import com.salesforce.graph.ApexPath; +import com.salesforce.rules.Violation; +import com.salesforce.testutils.DummyVertex; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class ProgressListenerImplTest { + + final ProgressListenerImpl progressListener; + + public ProgressListenerImplTest() { + SfgeConfigTestProvider.set( + new TestSfgeConfig() { + @Override + public int getProgressIncrementsOnVerbose() { + return 3; + } + }); + + progressListener = (ProgressListenerImpl) ProgressListenerProvider.get(); + } + + @BeforeEach + public void beforeEach() { + progressListener.reset(); + } + + @Test + public void testCompiledAnotherFile() { + progressListener.compiledAnotherFile(); + assertThat(progressListener.getFilesCompiled(), equalTo(1)); + } + + @Test + public void testFinishedAnalyzingEntryPoint() { + List paths = List.of(new ApexPath(null), new ApexPath(null)); + Set violations = + Set.of(new Violation.InternalErrorViolation("details", new DummyVertex("label"))); + + progressListener.finishedAnalyzingEntryPoint(paths, violations); + assertThat(progressListener.getPathsDetected(), equalTo(2)); + assertThat(progressListener.getViolationsDetected(), equalTo(1)); + assertThat(progressListener.getEntryPointsAnalyzed(), equalTo(1)); + assertThat(progressListener.getLastPathCountReported(), equalTo(0)); + } + + @Test + public void testFinishedAnalyzingEntryPoint_progressIncrement() { + List paths = List.of(new ApexPath(null), new ApexPath(null)); + Set violations = + Set.of(new Violation.InternalErrorViolation("details", new DummyVertex("label"))); + + progressListener.finishedAnalyzingEntryPoint(paths, violations); + assertThat(progressListener.getPathsDetected(), equalTo(2)); + assertThat(progressListener.getViolationsDetected(), equalTo(1)); + assertThat(progressListener.getLastPathCountReported(), equalTo(0)); + + progressListener.finishedAnalyzingEntryPoint(paths, violations); + assertThat(progressListener.getPathsDetected(), equalTo(4)); + assertThat(progressListener.getViolationsDetected(), equalTo(2)); + assertThat(progressListener.getEntryPointsAnalyzed(), equalTo(2)); + assertThat(progressListener.getLastPathCountReported(), equalTo(4)); + } + + @Test + public void testStringify() { + final List items = List.of("one", "two"); + assertThat(progressListener.stringify(items), equalTo("one,two")); + } + + @Test + public void testStringifyEmpty() { + final List items = new ArrayList<>(); + assertThat(progressListener.stringify(items), equalTo(ProgressListenerImpl.NONE_FOUND)); + } +} diff --git a/src/lib/services/CommandLineSupport.ts b/src/lib/services/CommandLineSupport.ts index b5842bba3..db7515672 100644 --- a/src/lib/services/CommandLineSupport.ts +++ b/src/lib/services/CommandLineSupport.ts @@ -77,12 +77,12 @@ export abstract class CommandLineSupport extends AsyncCreatable { // When data is passed back up to us, pop it onto the appropriate string. cp.stdout.on('data', data => { - if (!this.handleLiveOut(''+data)) { + if (!this.handleLiveOut(String(data))) { stdout += data; } }); cp.stderr.on('data', data => { - if (!this.handleLiveErr(''+data)) { + if (!this.handleLiveErr(String(data))) { stderr += data; } }); @@ -111,6 +111,7 @@ export abstract class CommandLineSupport extends AsyncCreatable { * @param data that was received as stdout * @returns true if data was handled and false if it needs to be handled later */ + /* eslint-disable-next-line @typescript-eslint/no-unused-vars */ protected handleLiveOut(data: string): boolean { // By default, we handle all data at the end return false; @@ -122,6 +123,7 @@ export abstract class CommandLineSupport extends AsyncCreatable { * @param err that was received as on stderr * @returns true if data was handled and false if it needs to be handled later */ + /* eslint-disable-next-line @typescript-eslint/no-unused-vars */ protected handleLiveErr(err: string): boolean { // By default, we handle all the errors at the end return false; diff --git a/src/lib/services/OutputProcessor.ts b/src/lib/services/OutputProcessor.ts index a9bf30ddd..720bf12f9 100644 --- a/src/lib/services/OutputProcessor.ts +++ b/src/lib/services/OutputProcessor.ts @@ -100,7 +100,6 @@ export class OutputProcessor extends AsyncCreatable { private getEventsFromString(str: string, startTag: string, endTag: string): RuleEvent[] { const events: RuleEvent[] = []; - // const regex = /SFDX-START(.*)SFDX-END/g; const regex = new RegExp(`^${startTag}(.*)${endTag}`, 'g'); const headerLength = startTag.length; const tailLength = endTag.length; diff --git a/test/commands/scanner/run.dfa.test.ts b/test/commands/scanner/run.dfa.test.ts new file mode 100644 index 000000000..9c8db76c8 --- /dev/null +++ b/test/commands/scanner/run.dfa.test.ts @@ -0,0 +1,81 @@ +import {expect} from '@salesforce/command/lib/test'; +import {setupCommandTest} from '../../TestUtils'; +import {Messages} from '@salesforce/core'; +import * as path from 'path'; + + +Messages.importMessagesDirectory(__dirname); +const sfgeMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'EventKeyTemplates'); + +const dfaTarget = path.join('test', 'code-fixtures', 'projects', 'sfge-smoke-app', 'src'); +const projectdir = path.join('test', 'code-fixtures', 'projects', 'sfge-smoke-app', 'src'); + +const apexControllerStr = 'UnsafeVfController'; +const customSettingsStr = 'none found'; +const fileCount = '7'; +const entryPointCount = '5'; +const pathCount = '6'; +const violationCount = '2'; + +const customSettingsMessage = sfgeMessages.getMessage('info.sfgeMetaInfoCollected', ['Custom Settings', customSettingsStr]); +const apexControllerMessage = sfgeMessages.getMessage('info.sfgeMetaInfoCollected', ['Apex Controllers', apexControllerStr]); +const compiledMessage = sfgeMessages.getMessage('info.sfgeFinishedCompilingFiles', [fileCount]); +const startGraphBuildMessage = sfgeMessages.getMessage('info.sfgeStartedBuildingGraph'); +const endGraphBuildMessage = sfgeMessages.getMessage('info.sfgeFinishedBuildingGraph'); +const identifiedEntryMessage = sfgeMessages.getMessage('info.sfgePathEntryPointsIdentified', [entryPointCount]); +const completedAnalysisMessage = sfgeMessages.getMessage('info.sfgeCompletedPathAnalysis', [pathCount, entryPointCount, violationCount]); + +function isSubstr(output: string, substring: string): boolean { + const regex = new RegExp(`^${substring}`, 'gm'); + const regexMatch = output.match(regex); + return regexMatch != null && regexMatch.length >= 1; +} + +function verifyContains(output: string, substring: string): void { + expect(isSubstr(output, substring), `Output "${output}" should contain substring "${substring}"`).is.true; +} + +function verifyNotContains(output: string, substring: string): void { + expect(isSubstr(output, substring), `Output "${output}" should not contain substring "${substring}"`).is.false; +} + +describe('scanner:run:dfa', function () { + describe('End to end', () => { + describe('--verbose', () => { + setupCommandTest + .command(['scanner:run:dfa', + '--target', dfaTarget, + '--projectdir', projectdir, + '--format', 'json' + ]) + .it('contains valid information when --verbose is not used', ctx => { + const output = ctx.stdout; + verifyNotContains(output, customSettingsMessage); + verifyNotContains(output, apexControllerMessage); + verifyNotContains(output, compiledMessage); + verifyNotContains(output, startGraphBuildMessage); + verifyNotContains(output, endGraphBuildMessage); + verifyNotContains(output, identifiedEntryMessage); + verifyContains(output, completedAnalysisMessage); + }); + + setupCommandTest + .command(['scanner:run:dfa', + '--target', dfaTarget, + '--projectdir', projectdir, + '--format', 'json', + '--verbose' + ]) + .it('contains valid information with --verbose', ctx => { + const output = ctx.stdout; + verifyContains(output, customSettingsMessage); + verifyContains(output, apexControllerMessage); + verifyContains(output, compiledMessage); + verifyContains(output, startGraphBuildMessage); + verifyContains(output, endGraphBuildMessage); + verifyContains(output, identifiedEntryMessage); + verifyContains(output, completedAnalysisMessage); + }); + }); + }); +}); \ No newline at end of file From 3b5a855107277b45f1263d80aa0f400ed7b567f9 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Thu, 11 Aug 2022 14:36:41 -0700 Subject: [PATCH 05/13] Improving javadocs --- .../rules/ops/ProgressListener.java | 33 +++++++++++++++++++ .../rules/ops/ProgressListenerImpl.java | 4 ++- .../rules/ops/ProgressListenerProvider.java | 3 ++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java index 7d56975a6..f6f2d1847 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java @@ -6,20 +6,53 @@ import java.util.Set; import java.util.TreeSet; +/** + * Observer that listens to activities happening in SFGE while analyzing source code. + */ public interface ProgressListener { + /** + * Invoked when meta information about source code is collected + * @param metaInfoType type of information + * @param itemsCollected items that were collected + */ void collectedMetaInfo(String metaInfoType, TreeSet itemsCollected); + /** + * Invoked when a file is compiled by Jorje. + */ void compiledAnotherFile(); + /** + * Invoked when compilation is completed for all files. + */ void finishedFileCompilation(); + /** + * Invoked when SFGE starts building the graph with information + * found during compilation. + */ void startedBuildingGraph(); + /** + * Invoked when SFGE completes building graph. + */ void completedBuildingGraph(); + /** + * Invoked when entry points to analysis are identified. + * @param pathEntryPointsCount number of entry points identified. + */ void pathEntryPointsIdentified(int pathEntryPointsCount); + /** + * Invoked when all the paths originating from an entry point are analyzed. + * @param paths number of paths that originated from an entry point. + * @param violations number of violations detected while walking the identified paths. + */ void finishedAnalyzingEntryPoint(List paths, Set violations); + /** + * Invoked when analysis is finished. + */ void completedAnalysis(); } diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java index 3f9989032..ec0bb128d 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java @@ -12,7 +12,9 @@ import java.util.Set; import java.util.TreeSet; -/** Publishes information to CLI on the progress of analysis. */ +/** + * Publishes realtime information to CLI on the progress of analysis. + */ public class ProgressListenerImpl implements ProgressListener { @VisibleForTesting static final String NONE_FOUND = "none found"; diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java index 379da954d..4ee1f0b4a 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java @@ -2,6 +2,9 @@ import com.google.common.annotations.VisibleForTesting; +/** + * Provides the singleton instance of {@link ProgressListener} in a thread-safe way. + */ public class ProgressListenerProvider { @VisibleForTesting static final ThreadLocal PROGRESS_LISTENER = From 88bd84f639f6cb9b5c3aa20c6a00ca6ae2f09dec Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Mon, 15 Aug 2022 12:54:29 -0700 Subject: [PATCH 06/13] Modifying spinner message to display SFGE progress --- .../com/salesforce/messaging/EventKey.java | 10 +++---- .../com/salesforce/messaging/Message.java | 1 + messages/EventKeyTemplates.js | 2 +- .../java/com/salesforce/config/EnvUtil.java | 2 +- .../rules/ops/ProgressListener.java | 28 ++++++------------- .../rules/ops/ProgressListenerImpl.java | 11 ++++---- .../rules/ops/ProgressListenerProvider.java | 4 +-- src/lib/ScannerCommand.ts | 8 ++++++ src/lib/ScannerEvents.ts | 1 + src/lib/services/OutputProcessor.ts | 17 ++++++++--- src/lib/sfge/SfgeWrapper.ts | 5 ++-- test/commands/scanner/run.dfa.test.ts | 17 +++++++++++ 12 files changed, 66 insertions(+), 40 deletions(-) diff --git a/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java b/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java index 18c0e722b..aaa621f5e 100644 --- a/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java +++ b/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java @@ -29,12 +29,12 @@ public enum EventKey { /** SFGE RELATED **/ INFO_GENERAL("info.sfgeInfoLog", 1, MessageType.INFO, MessageHandler.UX, true), INFO_META_INFO_COLLECTED("info.sfgeMetaInfoCollected", 2, MessageType.INFO, MessageHandler.UX, true), - INFO_COMPLETED_FILE_COMPILATION("info.sfgeFinishedCompilingFiles", 1, MessageType.INFO, MessageHandler.UX, true), - INFO_STARTED_BUILDING_GRAPH("info.sfgeStartedBuildingGraph", 0, MessageType.INFO, MessageHandler.UX, true), - INFO_COMPLETED_BUILDING_GRAPH("info.sfgeFinishedBuildingGraph", 0, MessageType.INFO, MessageHandler.UX, true), + INFO_COMPLETED_FILE_COMPILATION("info.sfgeFinishedCompilingFiles", 1, MessageType.INFO, MessageHandler.UX, false), + INFO_STARTED_BUILDING_GRAPH("info.sfgeStartedBuildingGraph", 0, MessageType.INFO, MessageHandler.UX_SPINNER, false), + INFO_COMPLETED_BUILDING_GRAPH("info.sfgeFinishedBuildingGraph", 0, MessageType.INFO, MessageHandler.UX_SPINNER, false), INFO_PATH_ENTRY_POINTS_IDENTIFIED("info.sfgePathEntryPointsIdentified", 1, MessageType.INFO, MessageHandler.UX, true), - INFO_PATH_ANALYSIS_PROGRESS("info.sfgeViolationsInPathProgress", 3, MessageType.INFO, MessageHandler.UX, true), - INFO_COMPLETED_PATH_ANALYSIS("info.sfgeCompletedPathAnalysis", 3, MessageType.INFO, MessageHandler.UX, false), + INFO_PATH_ANALYSIS_PROGRESS("info.sfgeViolationsInPathProgress", 4, MessageType.INFO, MessageHandler.UX_SPINNER, false), + INFO_COMPLETED_PATH_ANALYSIS("info.sfgeCompletedPathAnalysis", 3, MessageType.INFO, MessageHandler.UX_SPINNER, false), WARNING_GENERAL("warning.sfgeWarnLog", 1, MessageType.WARNING, MessageHandler.UX, true), WARNING_MULTIPLE_METHOD_TARGET_MATCHES("warning.multipleMethodTargetMatches", 3, MessageType.WARNING, MessageHandler.UX, false), WARNING_NO_METHOD_TARGET_MATCHES("warning.noMethodTargetMatches", 2, MessageType.WARNING, MessageHandler.UX, false), diff --git a/cli-messaging/src/main/java/com/salesforce/messaging/Message.java b/cli-messaging/src/main/java/com/salesforce/messaging/Message.java index 4a71e20cb..b5df0e39e 100644 --- a/cli-messaging/src/main/java/com/salesforce/messaging/Message.java +++ b/cli-messaging/src/main/java/com/salesforce/messaging/Message.java @@ -52,6 +52,7 @@ public long getTime() { enum MessageHandler { UX, + UX_SPINNER, INTERNAL } diff --git a/messages/EventKeyTemplates.js b/messages/EventKeyTemplates.js index 80300d7a0..14b7f6a2b 100644 --- a/messages/EventKeyTemplates.js +++ b/messages/EventKeyTemplates.js @@ -14,7 +14,7 @@ module.exports = { "sfgeStartedBuildingGraph": "Building graph . . .", "sfgeFinishedBuildingGraph": "Added all compilation units to graph.", "sfgePathEntryPointsIdentified": "Identified %s path entry point(s).", - "sfgeViolationsInPathProgress": "Detected %s violation(s) from %s path(s) on %s entry point(s) so far.", + "sfgeViolationsInPathProgress": "Detected %s violation(s) from %s path(s) on %s/%s entry point(s).", "sfgeCompletedPathAnalysis": "Overall, analyzed %s path(s) from %s entry point(s). Detected %s violation(s)." }, "warning": { diff --git a/sfge/src/main/java/com/salesforce/config/EnvUtil.java b/sfge/src/main/java/com/salesforce/config/EnvUtil.java index 7ff3a07db..456a90857 100644 --- a/sfge/src/main/java/com/salesforce/config/EnvUtil.java +++ b/sfge/src/main/java/com/salesforce/config/EnvUtil.java @@ -25,7 +25,7 @@ public final class EnvUtil { @VisibleForTesting static final boolean DEFAULT_RULE_ENABLE_WARNING_VIOLATION = true; @VisibleForTesting static final boolean DEFAULT_IGNORE_PARSE_ERRORS = false; @VisibleForTesting static final boolean DEFAULT_LOG_WARNINGS_ON_VERBOSE = false; - @VisibleForTesting static final int DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE = 50; + @VisibleForTesting static final int DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE = 10; /** * Returns the value of the {@link #ENV_RULE_THREAD_COUNT} environment variable if set, else diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java index f6f2d1847..2cfbd727a 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListener.java @@ -6,53 +6,43 @@ import java.util.Set; import java.util.TreeSet; -/** - * Observer that listens to activities happening in SFGE while analyzing source code. - */ +/** Observer that listens to activities happening in SFGE while analyzing source code. */ public interface ProgressListener { /** * Invoked when meta information about source code is collected + * * @param metaInfoType type of information * @param itemsCollected items that were collected */ void collectedMetaInfo(String metaInfoType, TreeSet itemsCollected); - /** - * Invoked when a file is compiled by Jorje. - */ + /** Invoked when a file is compiled by Jorje. */ void compiledAnotherFile(); - /** - * Invoked when compilation is completed for all files. - */ + /** Invoked when compilation is completed for all files. */ void finishedFileCompilation(); - /** - * Invoked when SFGE starts building the graph with information - * found during compilation. - */ + /** Invoked when SFGE starts building the graph with information found during compilation. */ void startedBuildingGraph(); - /** - * Invoked when SFGE completes building graph. - */ + /** Invoked when SFGE completes building graph. */ void completedBuildingGraph(); /** * Invoked when entry points to analysis are identified. + * * @param pathEntryPointsCount number of entry points identified. */ void pathEntryPointsIdentified(int pathEntryPointsCount); /** * Invoked when all the paths originating from an entry point are analyzed. + * * @param paths number of paths that originated from an entry point. * @param violations number of violations detected while walking the identified paths. */ void finishedAnalyzingEntryPoint(List paths, Set violations); - /** - * Invoked when analysis is finished. - */ + /** Invoked when analysis is finished. */ void completedAnalysis(); } diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java index ec0bb128d..304d5d687 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java @@ -12,9 +12,7 @@ import java.util.Set; import java.util.TreeSet; -/** - * Publishes realtime information to CLI on the progress of analysis. - */ +/** Publishes realtime information to CLI on the progress of analysis. */ public class ProgressListenerImpl implements ProgressListener { @VisibleForTesting static final String NONE_FOUND = "none found"; @@ -24,6 +22,7 @@ public class ProgressListenerImpl implements ProgressListener { private int lastPathCountReported = 0; private int violationsDetected = 0; private int entryPointsAnalyzed = 0; + private int totalEntryPoints = 0; private final int progressIncrements; @@ -74,10 +73,11 @@ public void completedBuildingGraph() { @Override public void pathEntryPointsIdentified(int pathEntryPointsCount) { + totalEntryPoints = pathEntryPointsCount; CliMessager.postMessage( "Path entry points identified", EventKey.INFO_PATH_ENTRY_POINTS_IDENTIFIED, - String.valueOf(pathEntryPointsCount)); + String.valueOf(totalEntryPoints)); } @Override @@ -94,7 +94,8 @@ public void finishedAnalyzingEntryPoint(List paths, Set vio EventKey.INFO_PATH_ANALYSIS_PROGRESS, String.valueOf(violationsDetected), String.valueOf(pathsDetected), - String.valueOf(entryPointsAnalyzed)); + String.valueOf(entryPointsAnalyzed), + String.valueOf(totalEntryPoints)); lastPathCountReported = pathsDetected; } diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java index 4ee1f0b4a..e50cd0868 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerProvider.java @@ -2,9 +2,7 @@ import com.google.common.annotations.VisibleForTesting; -/** - * Provides the singleton instance of {@link ProgressListener} in a thread-safe way. - */ +/** Provides the singleton instance of {@link ProgressListener} in a thread-safe way. */ public class ProgressListenerProvider { @VisibleForTesting static final ThreadLocal PROGRESS_LISTENER = diff --git a/src/lib/ScannerCommand.ts b/src/lib/ScannerCommand.ts index 1697ca0c0..73008d17f 100644 --- a/src/lib/ScannerCommand.ts +++ b/src/lib/ScannerCommand.ts @@ -86,6 +86,13 @@ export abstract class ScannerCommand extends SfdxCommand { this.ux.setSpinnerStatus(msg); } + /* eslint-disable-next-line @typescript-eslint/no-unused-vars */ + protected waitOnSpinner(msg: string): void { + // msg variable is thrown away - please don't send anything here. + const currentStatus = this.ux.getSpinnerStatus(); + this.ux.setSpinnerStatus(currentStatus + ' .'); + } + protected stopSpinner(msg: string): void { this.ux.stopSpinner(msg); } @@ -104,6 +111,7 @@ export abstract class ScannerCommand extends SfdxCommand { uxEvents.on(EVENTS.ERROR_VERBOSE, (msg: string) => this.displayError(msg)); uxEvents.on(EVENTS.START_SPINNER, (msg: string, status: string) => this.startSpinner(msg, status)); uxEvents.on(EVENTS.UPDATE_SPINNER, (msg: string) => this.updateSpinner(msg)); + uxEvents.on(EVENTS.WAIT_ON_SPINNER, (msg: string) => this.waitOnSpinner(msg)); uxEvents.on(EVENTS.STOP_SPINNER, (msg: string) => this.stopSpinner(msg)); } } diff --git a/src/lib/ScannerEvents.ts b/src/lib/ScannerEvents.ts index 1ddb1355f..4d5bbd36d 100644 --- a/src/lib/ScannerEvents.ts +++ b/src/lib/ScannerEvents.ts @@ -14,5 +14,6 @@ export enum EVENTS { // and `ux.stopSpinner()`, which are both no-ops if there's not an active spinner. START_SPINNER = 'start-spinner', UPDATE_SPINNER = 'update-spinner', + WAIT_ON_SPINNER = 'wait-on-spinner', STOP_SPINNER = 'stop-spinner' } diff --git a/src/lib/services/OutputProcessor.ts b/src/lib/services/OutputProcessor.ts index 720bf12f9..cf13229c2 100644 --- a/src/lib/services/OutputProcessor.ts +++ b/src/lib/services/OutputProcessor.ts @@ -1,7 +1,7 @@ import {Logger, LoggerLevel, Messages} from '@salesforce/core'; import {AsyncCreatable} from '@salesforce/kit'; import {RuleEvent} from '../../types'; -import {uxEvents} from '../ScannerEvents'; +import {EVENTS, uxEvents} from '../ScannerEvents'; Messages.importMessagesDirectory(__dirname); @@ -74,16 +74,25 @@ export class OutputProcessor extends AsyncCreatable { // Iterate over all of the events and throw them as appropriate. outEvents.forEach((event) => { this.logEvent(event); - if (event.handler === 'UX' || (event.handler === 'INTERNAL' && event.type === 'ERROR')) { - const eventType = `${event.type.toLowerCase()}-${event.verbose ? 'verbose' : 'always'}`; - this.emitUxEvent(eventType, event.messageKey, event.args); + let eventType = ''; + if (event.handler === 'UX_SPINNER') { + eventType = EVENTS.UPDATE_SPINNER.toString(); + } else if (event.handler === 'UX' || (event.handler === 'INTERNAL' && event.type === 'ERROR')) { + eventType = `${event.type.toLowerCase()}-${event.verbose ? 'verbose' : 'always'}`; } + this.emitUxEvent(eventType, event.messageKey, event.args); }); return true; } private emitUxEvent(eventType: string, messageKey: string, args: string[]): void { + if (eventType === '') { + this.logger.trace(`No event type requested for message ${messageKey}`); + return; + } + + this.logger.trace(`Sending new event of type ${eventType} and message ${messageKey}`); let constructedMessage: string = null; try { diff --git a/src/lib/sfge/SfgeWrapper.ts b/src/lib/sfge/SfgeWrapper.ts index 16e4cf700..9c99078af 100644 --- a/src/lib/sfge/SfgeWrapper.ts +++ b/src/lib/sfge/SfgeWrapper.ts @@ -63,11 +63,12 @@ class SfgeSpinnerManager extends AsyncCreatable implements SpinnerManager { } public startSpinner(): void { - uxEvents.emit(EVENTS.START_SPINNER, `Evaluating SFGE rules. See ${this.logFilePath} for logs`, "Please wait"); + uxEvents.emit(EVENTS.INFO_ALWAYS, `See ${this.logFilePath} for SFGE's logs`); + uxEvents.emit(EVENTS.START_SPINNER, `Analyzing with SFGE.`, "Please wait"); let intervalCount = 0; this.intervalId = setInterval(() => { - uxEvents.emit(EVENTS.UPDATE_SPINNER, "Please wait." + ".".repeat(intervalCount)); + uxEvents.emit(EVENTS.WAIT_ON_SPINNER, 'message is unused'); intervalCount += 1; }, 30000); } diff --git a/test/commands/scanner/run.dfa.test.ts b/test/commands/scanner/run.dfa.test.ts index 9c8db76c8..119d4a2b5 100644 --- a/test/commands/scanner/run.dfa.test.ts +++ b/test/commands/scanner/run.dfa.test.ts @@ -77,5 +77,22 @@ describe('scanner:run:dfa', function () { verifyContains(output, completedAnalysisMessage); }); }); + + describe('run with format --json', () => { + setupCommandTest + .command(['scanner:run:dfa', + '--target', dfaTarget, + '--projectdir', projectdir, + '--format', 'json' + ]) + .it('provides only json in stdout', ctx => { + try { + JSON.parse(ctx.stdout); + } catch (error) { + expect.fail("Invalid JSON output from --format json: " + ctx.stdout, error); + } + + }); + }); }); }); \ No newline at end of file From 978e49c3b02d3115dc3ec64066f227b0f8a373f2 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Mon, 15 Aug 2022 15:24:32 -0700 Subject: [PATCH 07/13] PR feedback + fixed end to end tests --- .../com/salesforce/messaging/EventKey.java | 4 +- messages/EventKeyTemplates.js | 2 +- .../java/com/salesforce/config/EnvUtil.java | 14 ++-- .../com/salesforce/config/SfgeConfig.java | 2 +- .../com/salesforce/config/SfgeConfigImpl.java | 4 +- .../rules/ops/ProgressListenerImpl.java | 2 +- .../config/SfgeConfigProviderTest.java | 11 ++-- .../com/salesforce/config/TestSfgeConfig.java | 4 +- .../rules/ops/ProgressListenerImplTest.java | 2 +- src/lib/sfge/SfgeWrapper.ts | 6 +- test/commands/scanner/run.dfa.test.ts | 65 +++++++++++-------- 11 files changed, 62 insertions(+), 54 deletions(-) diff --git a/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java b/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java index aaa621f5e..2b3c77685 100644 --- a/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java +++ b/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java @@ -29,10 +29,10 @@ public enum EventKey { /** SFGE RELATED **/ INFO_GENERAL("info.sfgeInfoLog", 1, MessageType.INFO, MessageHandler.UX, true), INFO_META_INFO_COLLECTED("info.sfgeMetaInfoCollected", 2, MessageType.INFO, MessageHandler.UX, true), - INFO_COMPLETED_FILE_COMPILATION("info.sfgeFinishedCompilingFiles", 1, MessageType.INFO, MessageHandler.UX, false), + INFO_COMPLETED_FILE_COMPILATION("info.sfgeFinishedCompilingFiles", 1, MessageType.INFO, MessageHandler.UX_SPINNER, false), INFO_STARTED_BUILDING_GRAPH("info.sfgeStartedBuildingGraph", 0, MessageType.INFO, MessageHandler.UX_SPINNER, false), INFO_COMPLETED_BUILDING_GRAPH("info.sfgeFinishedBuildingGraph", 0, MessageType.INFO, MessageHandler.UX_SPINNER, false), - INFO_PATH_ENTRY_POINTS_IDENTIFIED("info.sfgePathEntryPointsIdentified", 1, MessageType.INFO, MessageHandler.UX, true), + INFO_PATH_ENTRY_POINTS_IDENTIFIED("info.sfgePathEntryPointsIdentified", 1, MessageType.INFO, MessageHandler.UX_SPINNER, false), INFO_PATH_ANALYSIS_PROGRESS("info.sfgeViolationsInPathProgress", 4, MessageType.INFO, MessageHandler.UX_SPINNER, false), INFO_COMPLETED_PATH_ANALYSIS("info.sfgeCompletedPathAnalysis", 3, MessageType.INFO, MessageHandler.UX_SPINNER, false), WARNING_GENERAL("warning.sfgeWarnLog", 1, MessageType.WARNING, MessageHandler.UX, true), diff --git a/messages/EventKeyTemplates.js b/messages/EventKeyTemplates.js index 14b7f6a2b..5206ccc06 100644 --- a/messages/EventKeyTemplates.js +++ b/messages/EventKeyTemplates.js @@ -11,7 +11,7 @@ module.exports = { "sfgeInfoLog": "%s", "sfgeMetaInfoCollected": "Loaded %s: [ %s ]", "sfgeFinishedCompilingFiles": "Compiled %s files.", - "sfgeStartedBuildingGraph": "Building graph . . .", + "sfgeStartedBuildingGraph": "Building graph.", "sfgeFinishedBuildingGraph": "Added all compilation units to graph.", "sfgePathEntryPointsIdentified": "Identified %s path entry point(s).", "sfgeViolationsInPathProgress": "Detected %s violation(s) from %s path(s) on %s/%s entry point(s).", diff --git a/sfge/src/main/java/com/salesforce/config/EnvUtil.java b/sfge/src/main/java/com/salesforce/config/EnvUtil.java index 456a90857..afe45194f 100644 --- a/sfge/src/main/java/com/salesforce/config/EnvUtil.java +++ b/sfge/src/main/java/com/salesforce/config/EnvUtil.java @@ -10,8 +10,7 @@ public final class EnvUtil { "SFGE_RULE_ENABLE_WARNING_VIOLATION"; private static final String ENV_IGNORE_PARSE_ERRORS = "SFGE_IGNORE_PARSE_ERRORS"; private static final String ENV_LOG_WARNINGS_ON_VERBOSE = "SFGE_LOG_WARNINGS_ON_VERBOSE"; - private static final String ENV_PROGRESS_INCREMENTS_ON_VERBOSE = - "SFGE_PROGRESS_INCREMENTS_ON_VERBOSE"; + private static final String ENV_PROGRESS_INCREMENTS = "SFGE_PROGRESS_INCREMENTS"; // TODO: These should move to SfgeConfigImpl and this class should return Optionals @VisibleForTesting @@ -25,7 +24,7 @@ public final class EnvUtil { @VisibleForTesting static final boolean DEFAULT_RULE_ENABLE_WARNING_VIOLATION = true; @VisibleForTesting static final boolean DEFAULT_IGNORE_PARSE_ERRORS = false; @VisibleForTesting static final boolean DEFAULT_LOG_WARNINGS_ON_VERBOSE = false; - @VisibleForTesting static final int DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE = 10; + @VisibleForTesting static final int DEFAULT_PROGRESS_INCREMENTS = 10; /** * Returns the value of the {@link #ENV_RULE_THREAD_COUNT} environment variable if set, else @@ -80,12 +79,11 @@ static boolean shouldLogWarningsOnVerbose() { * Gets the level of increments at which path analysis progress update is provided. Applicable * only with --verbose. * - * @return value of {@link #ENV_PROGRESS_INCREMENTS_ON_VERBOSE} environment variable if set, - * else {@link #DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE} + * @return value of {@link #ENV_PROGRESS_INCREMENTS} environment variable if set, else {@link + * #DEFAULT_PROGRESS_INCREMENTS} */ - static int getProgressIncrementsOnVerbose() { - return getIntOrDefault( - ENV_PROGRESS_INCREMENTS_ON_VERBOSE, DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE); + static int getProgressIncrements() { + return getIntOrDefault(ENV_PROGRESS_INCREMENTS, DEFAULT_PROGRESS_INCREMENTS); } private static int getIntOrDefault(String name, int defaultValue) { diff --git a/sfge/src/main/java/com/salesforce/config/SfgeConfig.java b/sfge/src/main/java/com/salesforce/config/SfgeConfig.java index 6e3d1e4d4..9cbc3da47 100644 --- a/sfge/src/main/java/com/salesforce/config/SfgeConfig.java +++ b/sfge/src/main/java/com/salesforce/config/SfgeConfig.java @@ -27,5 +27,5 @@ public interface SfgeConfig { * Should be used to set the level of increments at which path analysis progress update is * provided */ - int getProgressIncrementsOnVerbose(); + int getProgressIncrements(); } diff --git a/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java b/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java index 89f39c53a..fad94991f 100644 --- a/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java +++ b/sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java @@ -31,8 +31,8 @@ public boolean shouldLogWarningsOnVerbose() { } @Override - public int getProgressIncrementsOnVerbose() { - return EnvUtil.getProgressIncrementsOnVerbose(); + public int getProgressIncrements() { + return EnvUtil.getProgressIncrements(); } static SfgeConfigImpl getInstance() { diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java index 304d5d687..3e362f502 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java @@ -35,7 +35,7 @@ private static final class LazyHolder { } private ProgressListenerImpl() { - progressIncrements = SfgeConfigProvider.get().getProgressIncrementsOnVerbose(); + progressIncrements = SfgeConfigProvider.get().getProgressIncrements(); } @Override diff --git a/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java b/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java index 8402613a1..189d20a92 100644 --- a/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java +++ b/sfge/src/test/java/com/salesforce/config/SfgeConfigProviderTest.java @@ -48,8 +48,8 @@ public boolean shouldLogWarningsOnVerbose() { } @Override - public int getProgressIncrementsOnVerbose() { - return -1 * EnvUtil.DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE; + public int getProgressIncrements() { + return -1 * EnvUtil.DEFAULT_PROGRESS_INCREMENTS; } }); @@ -71,8 +71,8 @@ public int getProgressIncrementsOnVerbose() { sfgeConfig.shouldLogWarningsOnVerbose(), equalTo(!EnvUtil.DEFAULT_LOG_WARNINGS_ON_VERBOSE)); MatcherAssert.assertThat( - sfgeConfig.getProgressIncrementsOnVerbose(), - equalTo(-1 * EnvUtil.getProgressIncrementsOnVerbose())); + sfgeConfig.getProgressIncrements(), + equalTo(-1 * EnvUtil.getProgressIncrements())); } finally { SfgeConfigTestProvider.remove(); } @@ -100,7 +100,6 @@ private void assertDefaultImplementation(SfgeConfig sfgeConfig) { sfgeConfig.shouldLogWarningsOnVerbose(), equalTo(EnvUtil.DEFAULT_LOG_WARNINGS_ON_VERBOSE)); MatcherAssert.assertThat( - sfgeConfig.getProgressIncrementsOnVerbose(), - equalTo(EnvUtil.DEFAULT_PROGRESS_INCREMENTS_ON_VERBOSE)); + sfgeConfig.getProgressIncrements(), equalTo(EnvUtil.DEFAULT_PROGRESS_INCREMENTS)); } } diff --git a/sfge/src/test/java/com/salesforce/config/TestSfgeConfig.java b/sfge/src/test/java/com/salesforce/config/TestSfgeConfig.java index b705c4527..c6eecb136 100644 --- a/sfge/src/test/java/com/salesforce/config/TestSfgeConfig.java +++ b/sfge/src/test/java/com/salesforce/config/TestSfgeConfig.java @@ -31,7 +31,7 @@ public boolean shouldLogWarningsOnVerbose() { } @Override - public int getProgressIncrementsOnVerbose() { - return SfgeConfigImpl.getInstance().getProgressIncrementsOnVerbose(); + public int getProgressIncrements() { + return SfgeConfigImpl.getInstance().getProgressIncrements(); } } diff --git a/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java b/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java index c7a10b625..f3145c1f4 100644 --- a/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java +++ b/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java @@ -22,7 +22,7 @@ public ProgressListenerImplTest() { SfgeConfigTestProvider.set( new TestSfgeConfig() { @Override - public int getProgressIncrementsOnVerbose() { + public int getProgressIncrements() { return 3; } }); diff --git a/src/lib/sfge/SfgeWrapper.ts b/src/lib/sfge/SfgeWrapper.ts index 9c99078af..e216d7d57 100644 --- a/src/lib/sfge/SfgeWrapper.ts +++ b/src/lib/sfge/SfgeWrapper.ts @@ -63,13 +63,11 @@ class SfgeSpinnerManager extends AsyncCreatable implements SpinnerManager { } public startSpinner(): void { - uxEvents.emit(EVENTS.INFO_ALWAYS, `See ${this.logFilePath} for SFGE's logs`); - uxEvents.emit(EVENTS.START_SPINNER, `Analyzing with SFGE.`, "Please wait"); + uxEvents.emit(EVENTS.START_SPINNER, `Analyzing with SFGE. See ${this.logFilePath} for details.`, "Please wait"); - let intervalCount = 0; + // TODO: This timer logic should ideally live inside waitOnSpinner() this.intervalId = setInterval(() => { uxEvents.emit(EVENTS.WAIT_ON_SPINNER, 'message is unused'); - intervalCount += 1; }, 30000); } diff --git a/test/commands/scanner/run.dfa.test.ts b/test/commands/scanner/run.dfa.test.ts index 119d4a2b5..d46220036 100644 --- a/test/commands/scanner/run.dfa.test.ts +++ b/test/commands/scanner/run.dfa.test.ts @@ -2,6 +2,8 @@ import {expect} from '@salesforce/command/lib/test'; import {setupCommandTest} from '../../TestUtils'; import {Messages} from '@salesforce/core'; import * as path from 'path'; +import Dfa from '../../../src/commands/scanner/run/dfa'; +import * as sinon from 'sinon'; Messages.importMessagesDirectory(__dirname); @@ -26,7 +28,8 @@ const identifiedEntryMessage = sfgeMessages.getMessage('info.sfgePathEntryPoints const completedAnalysisMessage = sfgeMessages.getMessage('info.sfgeCompletedPathAnalysis', [pathCount, entryPointCount, violationCount]); function isSubstr(output: string, substring: string): boolean { - const regex = new RegExp(`^${substring}`, 'gm'); + const updatedSubstr = substring.replace('[', '\\['); + const regex = new RegExp(`^${updatedSubstr}`, 'gm'); const regexMatch = output.match(regex); return regexMatch != null && regexMatch.length >= 1; } @@ -40,8 +43,23 @@ function verifyNotContains(output: string, substring: string): void { } describe('scanner:run:dfa', function () { + this.timeout(10000); // TODO why do we get timeouts at the default of 5000? What is so expensive here? + describe('End to end', () => { - describe('--verbose', () => { + + describe('Progress output', () => { + let sandbox; + let spy: sinon.SinonSpy; + before(() => { + sandbox = sinon.createSandbox(); + spy = sandbox.spy(Dfa.prototype, "updateSpinner"); + }); + + after(() => { + spy.restore(); + sinon.restore(); + }); + setupCommandTest .command(['scanner:run:dfa', '--target', dfaTarget, @@ -52,11 +70,7 @@ describe('scanner:run:dfa', function () { const output = ctx.stdout; verifyNotContains(output, customSettingsMessage); verifyNotContains(output, apexControllerMessage); - verifyNotContains(output, compiledMessage); - verifyNotContains(output, startGraphBuildMessage); - verifyNotContains(output, endGraphBuildMessage); - verifyNotContains(output, identifiedEntryMessage); - verifyContains(output, completedAnalysisMessage); + expect(spy.calledWith(compiledMessage, startGraphBuildMessage, endGraphBuildMessage, identifiedEntryMessage, completedAnalysisMessage)); }); setupCommandTest @@ -70,29 +84,28 @@ describe('scanner:run:dfa', function () { const output = ctx.stdout; verifyContains(output, customSettingsMessage); verifyContains(output, apexControllerMessage); - verifyContains(output, compiledMessage); - verifyContains(output, startGraphBuildMessage); - verifyContains(output, endGraphBuildMessage); - verifyContains(output, identifiedEntryMessage); - verifyContains(output, completedAnalysisMessage); + expect(spy.calledWith(compiledMessage, startGraphBuildMessage, endGraphBuildMessage, identifiedEntryMessage, completedAnalysisMessage)); }); }); - describe('run with format --json', () => { - setupCommandTest - .command(['scanner:run:dfa', - '--target', dfaTarget, - '--projectdir', projectdir, - '--format', 'json' - ]) - .it('provides only json in stdout', ctx => { - try { - JSON.parse(ctx.stdout); - } catch (error) { - expect.fail("Invalid JSON output from --format json: " + ctx.stdout, error); - } - + describe('Output consistency', () => { + describe('run with format --json', () => { + setupCommandTest + .command(['scanner:run:dfa', + '--target', dfaTarget, + '--projectdir', projectdir, + '--format', 'json' + ]) + .it('provides only json in stdout', ctx => { + try { + JSON.parse(ctx.stdout); + } catch (error) { + expect.fail("dummy", "another dummy", "Invalid JSON output from --format json: " + ctx.stdout + ", error = " + error); + } + + }); }); }); + }); }); \ No newline at end of file From fd3af9c2df3edda53c30d2d5ad49d93d092d0047 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Mon, 15 Aug 2022 15:32:34 -0700 Subject: [PATCH 08/13] Reordered verbose test --- test/commands/scanner/run.dfa.test.ts | 40 +++++++++++++-------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/test/commands/scanner/run.dfa.test.ts b/test/commands/scanner/run.dfa.test.ts index d46220036..76ef08b90 100644 --- a/test/commands/scanner/run.dfa.test.ts +++ b/test/commands/scanner/run.dfa.test.ts @@ -43,10 +43,29 @@ function verifyNotContains(output: string, substring: string): void { } describe('scanner:run:dfa', function () { - this.timeout(10000); // TODO why do we get timeouts at the default of 5000? What is so expensive here? + this.timeout(20000); // TODO why do we get timeouts at the default of 5000? What is so expensive here? describe('End to end', () => { + describe('Output consistency', () => { + describe('run with --format json', () => { + setupCommandTest + .command(['scanner:run:dfa', + '--target', dfaTarget, + '--projectdir', projectdir, + '--format', 'json' + ]) + .it('provides only json in stdout', ctx => { + try { + JSON.parse(ctx.stdout); + } catch (error) { + expect.fail("dummy", "another dummy", "Invalid JSON output from --format json: " + ctx.stdout + ", error = " + error); + } + + }); + }); + }); + describe('Progress output', () => { let sandbox; let spy: sinon.SinonSpy; @@ -87,25 +106,6 @@ describe('scanner:run:dfa', function () { expect(spy.calledWith(compiledMessage, startGraphBuildMessage, endGraphBuildMessage, identifiedEntryMessage, completedAnalysisMessage)); }); }); - - describe('Output consistency', () => { - describe('run with format --json', () => { - setupCommandTest - .command(['scanner:run:dfa', - '--target', dfaTarget, - '--projectdir', projectdir, - '--format', 'json' - ]) - .it('provides only json in stdout', ctx => { - try { - JSON.parse(ctx.stdout); - } catch (error) { - expect.fail("dummy", "another dummy", "Invalid JSON output from --format json: " + ctx.stdout + ", error = " + error); - } - - }); - }); - }); }); }); \ No newline at end of file From 8fd20440f7e5209402533c6c9c71585976b8f7a5 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Mon, 15 Aug 2022 15:49:52 -0700 Subject: [PATCH 09/13] remove conditional on handleLiveOut --- src/lib/services/CommandLineSupport.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/lib/services/CommandLineSupport.ts b/src/lib/services/CommandLineSupport.ts index db7515672..19bf04599 100644 --- a/src/lib/services/CommandLineSupport.ts +++ b/src/lib/services/CommandLineSupport.ts @@ -77,14 +77,12 @@ export abstract class CommandLineSupport extends AsyncCreatable { // When data is passed back up to us, pop it onto the appropriate string. cp.stdout.on('data', data => { - if (!this.handleLiveOut(String(data))) { - stdout += data; - } + this.handleLiveOut(String(data)) + stdout += data; }); cp.stderr.on('data', data => { - if (!this.handleLiveErr(String(data))) { - stderr += data; - } + this.handleLiveErr(String(data)) + stderr += data; }); cp.on('exit', code => { From 5acf779a411ff3eea492d923bcb9feeb58e8b616 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Mon, 15 Aug 2022 16:04:38 -0700 Subject: [PATCH 10/13] Minor changes --- .../rules/ops/ProgressListenerImplTest.java | 12 ++++---- src/lib/services/CommandLineSupport.ts | 30 ++----------------- src/lib/services/OutputProcessor.ts | 4 --- src/lib/sfge/SfgeWrapper.ts | 7 ----- 4 files changed, 10 insertions(+), 43 deletions(-) diff --git a/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java b/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java index f3145c1f4..2782187d6 100644 --- a/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java +++ b/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java @@ -3,6 +3,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; import com.salesforce.config.SfgeConfigTestProvider; import com.salesforce.config.TestSfgeConfig; import com.salesforce.graph.ApexPath; @@ -43,9 +45,9 @@ public void testCompiledAnotherFile() { @Test public void testFinishedAnalyzingEntryPoint() { - List paths = List.of(new ApexPath(null), new ApexPath(null)); + List paths = Lists.newArrayList(new ApexPath(null), new ApexPath(null)); Set violations = - Set.of(new Violation.InternalErrorViolation("details", new DummyVertex("label"))); + Sets.newHashSet(new Violation.InternalErrorViolation("details", new DummyVertex("label"))); progressListener.finishedAnalyzingEntryPoint(paths, violations); assertThat(progressListener.getPathsDetected(), equalTo(2)); @@ -56,9 +58,9 @@ public void testFinishedAnalyzingEntryPoint() { @Test public void testFinishedAnalyzingEntryPoint_progressIncrement() { - List paths = List.of(new ApexPath(null), new ApexPath(null)); + List paths = Lists.newArrayList(new ApexPath(null), new ApexPath(null)); Set violations = - Set.of(new Violation.InternalErrorViolation("details", new DummyVertex("label"))); + Sets.newHashSet(new Violation.InternalErrorViolation("details", new DummyVertex("label"))); progressListener.finishedAnalyzingEntryPoint(paths, violations); assertThat(progressListener.getPathsDetected(), equalTo(2)); @@ -74,7 +76,7 @@ public void testFinishedAnalyzingEntryPoint_progressIncrement() { @Test public void testStringify() { - final List items = List.of("one", "two"); + final List items = Lists.newArrayList("one", "two"); assertThat(progressListener.stringify(items), equalTo("one,two")); } diff --git a/src/lib/services/CommandLineSupport.ts b/src/lib/services/CommandLineSupport.ts index 19bf04599..67e07ccee 100644 --- a/src/lib/services/CommandLineSupport.ts +++ b/src/lib/services/CommandLineSupport.ts @@ -28,7 +28,7 @@ export abstract class CommandLineSupport extends AsyncCreatable { private parentLogger: Logger; private parentInitialized: boolean; - protected outputProcessor: OutputProcessor; + private outputProcessor: OutputProcessor; protected async init(): Promise { @@ -77,11 +77,11 @@ export abstract class CommandLineSupport extends AsyncCreatable { // When data is passed back up to us, pop it onto the appropriate string. cp.stdout.on('data', data => { - this.handleLiveOut(String(data)) + this.outputProcessor.processRealtimeOutput(data); stdout += data; }); cp.stderr.on('data', data => { - this.handleLiveErr(String(data)) + this.outputProcessor.processRealtimeOutput(data); stderr += data; }); @@ -102,28 +102,4 @@ export abstract class CommandLineSupport extends AsyncCreatable { }); }); } - - /** - * Handles output in realtime. - * Individual engines can override this hook to have custom implementation. - * @param data that was received as stdout - * @returns true if data was handled and false if it needs to be handled later - */ - /* eslint-disable-next-line @typescript-eslint/no-unused-vars */ - protected handleLiveOut(data: string): boolean { - // By default, we handle all data at the end - return false; - } - - /** - * Handles error messages in realtime. - * Individual engines can override this hook to have custom implementation. - * @param err that was received as on stderr - * @returns true if data was handled and false if it needs to be handled later - */ - /* eslint-disable-next-line @typescript-eslint/no-unused-vars */ - protected handleLiveErr(err: string): boolean { - // By default, we handle all the errors at the end - return false; - } } diff --git a/src/lib/services/OutputProcessor.ts b/src/lib/services/OutputProcessor.ts index cf13229c2..bc0d6908c 100644 --- a/src/lib/services/OutputProcessor.ts +++ b/src/lib/services/OutputProcessor.ts @@ -35,10 +35,6 @@ export class OutputProcessor extends AsyncCreatable { this.initialized = true; } - public isRealtimeOutput(out: string): boolean { - return out.startsWith(REALTIME_MESSAGE_START_TAG); - } - public processOutput(out: string): boolean { return this.processAllOutput(out, MESSAGE_START_TAG, MESSAGE_END_TAG); } diff --git a/src/lib/sfge/SfgeWrapper.ts b/src/lib/sfge/SfgeWrapper.ts index e216d7d57..bf488d97c 100644 --- a/src/lib/sfge/SfgeWrapper.ts +++ b/src/lib/sfge/SfgeWrapper.ts @@ -229,11 +229,4 @@ export class SfgeWrapper extends CommandLineSupport { }); return wrapper.execute(); } - - protected handleLiveOut(data: string): boolean { - if (this.outputProcessor.isRealtimeOutput(data)) { - return this.outputProcessor.processRealtimeOutput(data); - } - return false; - } } From 805ca8c397b767186aafd9c4119e29be8bcec740 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Mon, 15 Aug 2022 16:25:36 -0700 Subject: [PATCH 11/13] Fixing issues introduced in previous commit --- src/lib/services/CommandLineSupport.ts | 4 ++-- src/lib/services/OutputProcessor.ts | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/lib/services/CommandLineSupport.ts b/src/lib/services/CommandLineSupport.ts index 67e07ccee..63a912f71 100644 --- a/src/lib/services/CommandLineSupport.ts +++ b/src/lib/services/CommandLineSupport.ts @@ -77,11 +77,11 @@ export abstract class CommandLineSupport extends AsyncCreatable { // When data is passed back up to us, pop it onto the appropriate string. cp.stdout.on('data', data => { - this.outputProcessor.processRealtimeOutput(data); + this.outputProcessor.processRealtimeOutput(String(data)); stdout += data; }); cp.stderr.on('data', data => { - this.outputProcessor.processRealtimeOutput(data); + this.outputProcessor.processRealtimeOutput(String(data)); stderr += data; }); diff --git a/src/lib/services/OutputProcessor.ts b/src/lib/services/OutputProcessor.ts index bc0d6908c..cf13229c2 100644 --- a/src/lib/services/OutputProcessor.ts +++ b/src/lib/services/OutputProcessor.ts @@ -35,6 +35,10 @@ export class OutputProcessor extends AsyncCreatable { this.initialized = true; } + public isRealtimeOutput(out: string): boolean { + return out.startsWith(REALTIME_MESSAGE_START_TAG); + } + public processOutput(out: string): boolean { return this.processAllOutput(out, MESSAGE_START_TAG, MESSAGE_END_TAG); } From 6776803aad442f3c3f1d994da97eb99338df4f5f Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Mon, 15 Aug 2022 21:12:41 -0700 Subject: [PATCH 12/13] Creating an alternate test provider for ProgressImpl --- .../rules/ops/ProgressListenerImpl.java | 3 +- .../com/salesforce/SfgeTestExtension.java | 3 ++ .../rules/ops/ProgressListenerImplTest.java | 10 ++-- .../ops/ProgressListenerTestProvider.java | 26 ++++++++++ .../rules/ops/TestProgressListenerImpl.java | 50 +++++++++++++++++++ 5 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerTestProvider.java create mode 100644 sfge/src/test/java/com/salesforce/rules/ops/TestProgressListenerImpl.java diff --git a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java index 3e362f502..a695113d1 100644 --- a/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java +++ b/sfge/src/main/java/com/salesforce/rules/ops/ProgressListenerImpl.java @@ -34,7 +34,8 @@ private static final class LazyHolder { private static final ProgressListenerImpl INSTANCE = new ProgressListenerImpl(); } - private ProgressListenerImpl() { + @VisibleForTesting + ProgressListenerImpl() { progressIncrements = SfgeConfigProvider.get().getProgressIncrements(); } diff --git a/sfge/src/test/java/com/salesforce/SfgeTestExtension.java b/sfge/src/test/java/com/salesforce/SfgeTestExtension.java index 36cf3450c..de875ad2a 100644 --- a/sfge/src/test/java/com/salesforce/SfgeTestExtension.java +++ b/sfge/src/test/java/com/salesforce/SfgeTestExtension.java @@ -4,6 +4,7 @@ import com.salesforce.graph.TestMetadataInfo; import com.salesforce.graph.cache.VertexCacheTestProvider; import com.salesforce.metainfo.MetaInfoCollectorTestProvider; +import com.salesforce.rules.ops.ProgressListenerTestProvider; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.junit.jupiter.api.extension.BeforeTestExecutionCallback; @@ -31,6 +32,7 @@ public void beforeTestExecution(ExtensionContext context) { MetadataInfoTestProvider.initializeForTest(); VertexCacheTestProvider.initializeForTest(); MetaInfoCollectorTestProvider.initializeForTest(); + ProgressListenerTestProvider.initializeForTest(); } /** @@ -45,5 +47,6 @@ public void preDestroyTestInstance(ExtensionContext context) { MetadataInfoTestProvider.remove(); VertexCacheTestProvider.remove(); MetaInfoCollectorTestProvider.remove(); + ProgressListenerTestProvider.remove(); } } diff --git a/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java b/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java index 2782187d6..5f5dff5e9 100644 --- a/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java +++ b/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerImplTest.java @@ -29,7 +29,8 @@ public int getProgressIncrements() { } }); - progressListener = (ProgressListenerImpl) ProgressListenerProvider.get(); + // Creating an instance directly to avoid conflicts with other tests + progressListener = new ProgressListenerImpl(); } @BeforeEach @@ -47,7 +48,8 @@ public void testCompiledAnotherFile() { public void testFinishedAnalyzingEntryPoint() { List paths = Lists.newArrayList(new ApexPath(null), new ApexPath(null)); Set violations = - Sets.newHashSet(new Violation.InternalErrorViolation("details", new DummyVertex("label"))); + Sets.newHashSet( + new Violation.InternalErrorViolation("details", new DummyVertex("label"))); progressListener.finishedAnalyzingEntryPoint(paths, violations); assertThat(progressListener.getPathsDetected(), equalTo(2)); @@ -60,11 +62,13 @@ public void testFinishedAnalyzingEntryPoint() { public void testFinishedAnalyzingEntryPoint_progressIncrement() { List paths = Lists.newArrayList(new ApexPath(null), new ApexPath(null)); Set violations = - Sets.newHashSet(new Violation.InternalErrorViolation("details", new DummyVertex("label"))); + Sets.newHashSet( + new Violation.InternalErrorViolation("details", new DummyVertex("label"))); progressListener.finishedAnalyzingEntryPoint(paths, violations); assertThat(progressListener.getPathsDetected(), equalTo(2)); assertThat(progressListener.getViolationsDetected(), equalTo(1)); + assertThat(progressListener.getEntryPointsAnalyzed(), equalTo(1)); assertThat(progressListener.getLastPathCountReported(), equalTo(0)); progressListener.finishedAnalyzingEntryPoint(paths, violations); diff --git a/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerTestProvider.java b/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerTestProvider.java new file mode 100644 index 000000000..c378e58c3 --- /dev/null +++ b/sfge/src/test/java/com/salesforce/rules/ops/ProgressListenerTestProvider.java @@ -0,0 +1,26 @@ +package com.salesforce.rules.ops; + +import com.salesforce.SfgeTestExtension; +import org.junit.jupiter.api.extension.ExtensionContext; + +/** + * Test implementation that overrides behavior of {@link ProgressListenerProvider}. Initializes a + * test with a unique {@link ProgressListener} implementation for the current thread. + */ +public class ProgressListenerTestProvider { + /** + * Create a new ProgressListenerImpl for the current thread. This is invoked from {@link + * SfgeTestExtension#beforeTestExecution(ExtensionContext)} for all tests. + */ + public static void initializeForTest() { + ProgressListenerProvider.PROGRESS_LISTENER.set(new TestProgressListenerImpl()); + } + + /** + * Remove the ProgressListener from the current thread. This is invoked from {@link + * SfgeTestExtension#preDestroyTestInstance(ExtensionContext)} for all tests. + */ + public static void remove() { + ProgressListenerProvider.PROGRESS_LISTENER.remove(); + } +} diff --git a/sfge/src/test/java/com/salesforce/rules/ops/TestProgressListenerImpl.java b/sfge/src/test/java/com/salesforce/rules/ops/TestProgressListenerImpl.java new file mode 100644 index 000000000..b0ca29ca5 --- /dev/null +++ b/sfge/src/test/java/com/salesforce/rules/ops/TestProgressListenerImpl.java @@ -0,0 +1,50 @@ +package com.salesforce.rules.ops; + +import com.salesforce.graph.ApexPath; +import com.salesforce.rules.Violation; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; + +/** Test implementation of {@link ProgressListener} to use as a non-singleton with tests */ +public class TestProgressListenerImpl implements ProgressListener { + @Override + public void collectedMetaInfo(String metaInfoType, TreeSet itemsCollected) { + // do nothing + } + + @Override + public void compiledAnotherFile() { + // do nothing + } + + @Override + public void finishedFileCompilation() { + // do nothing + } + + @Override + public void startedBuildingGraph() { + // do nothing + } + + @Override + public void completedBuildingGraph() { + // do nothing + } + + @Override + public void pathEntryPointsIdentified(int pathEntryPointsCount) { + // do nothing + } + + @Override + public void finishedAnalyzingEntryPoint(List paths, Set violations) { + // do nothing + } + + @Override + public void completedAnalysis() { + // do nothing + } +} From a995ba5f37a61641996f9ee4e9c334a26d102879 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Mon, 15 Aug 2022 22:07:56 -0700 Subject: [PATCH 13/13] --verbose in test issue --- test/commands/scanner/run.dfa.test.ts | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/test/commands/scanner/run.dfa.test.ts b/test/commands/scanner/run.dfa.test.ts index 76ef08b90..e51826c8c 100644 --- a/test/commands/scanner/run.dfa.test.ts +++ b/test/commands/scanner/run.dfa.test.ts @@ -34,9 +34,6 @@ function isSubstr(output: string, substring: string): boolean { return regexMatch != null && regexMatch.length >= 1; } -function verifyContains(output: string, substring: string): void { - expect(isSubstr(output, substring), `Output "${output}" should contain substring "${substring}"`).is.true; -} function verifyNotContains(output: string, substring: string): void { expect(isSubstr(output, substring), `Output "${output}" should not contain substring "${substring}"`).is.false; @@ -92,19 +89,6 @@ describe('scanner:run:dfa', function () { expect(spy.calledWith(compiledMessage, startGraphBuildMessage, endGraphBuildMessage, identifiedEntryMessage, completedAnalysisMessage)); }); - setupCommandTest - .command(['scanner:run:dfa', - '--target', dfaTarget, - '--projectdir', projectdir, - '--format', 'json', - '--verbose' - ]) - .it('contains valid information with --verbose', ctx => { - const output = ctx.stdout; - verifyContains(output, customSettingsMessage); - verifyContains(output, apexControllerMessage); - expect(spy.calledWith(compiledMessage, startGraphBuildMessage, endGraphBuildMessage, identifiedEntryMessage, completedAnalysisMessage)); - }); }); });