Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

57 / Удалил неиспользуемые объекты #65

Merged
merged 6 commits into from
Jan 15, 2025

Conversation

alkoleft
Copy link
Member

@alkoleft alkoleft commented Jan 15, 2025

Удалил мусор

Close #57

Summary by CodeRabbit

Примечания к выпуску

  • Новые функции

    • Добавлена поддержка модульных тестов YAxUnit
    • Обновлена система управления сеансами тестирования
  • Изменения в пользовательском интерфейсе

    • Переработаны сообщения и метки в интерфейсе
    • Упрощена навигация по результатам тестов
  • Технические улучшения

    • Оптимизирована работа с сеансами тестирования
    • Удалены устаревшие зависимости и константы
    • Обновлена система локализации сообщений
  • Исправления

    • Устранены проблемы с импортом и отображением тестов
    • Улучшена стабильность работы модуля тестирования

Copy link

coderabbitai bot commented Jan 15, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Данный pull request представляет собой обширное рефакторинг плагина тестирования, включающий переименование классов, изменение структуры сообщений и модернизацию управления сессиями тестирования. Основные изменения включают переход от специфических классов JUnit к более обобщенным компонентам, унификацию именования и оптимизацию обработки тестовых сессий.

Changes

Файлы Изменения
viewer/META-INF/MANIFEST.MF Удаление зависимости от org.apache.commons.lang3
viewer/plugin.properties Удаление устаревших записей, связанных с JUnit
viewer/plugin.xml Удаление расширений для настроек и ключевых слов
viewer/src/main/java/ru/biatech/edt/junit/model/ Рефакторинг классов управления сессиями тестирования
viewer/src/main/java/ru/biatech/edt/junit/ui/ Обновление источников сообщений с JUnitMessages на UIMessages

Assessment against linked issues

Objective Addressed Explanation
Убрать страницу параметров [#57] Удалены расширения и классы, связанные со страницей параметров

Poem

🐰 В мире кода, где тесты танцуют,
Старый JUnit прощально кивает.
Новый дизайн, легкий и смелый,
Сессии мчатся, как ветер веселый!
Рефакторинг - наш верный маршрут! 🚀


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 964e769 and 6303d34.

📒 Files selected for processing (52)
  • viewer/src/main/java/ru/biatech/edt/junit/BasicElementLabels.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/Core.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/TestViewerPlugin.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/lifecycle/LifecycleMonitor.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/ui/LaunchConfigurationTab.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/ui/LaunchControl.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/v8/LaunchConfigurationDelegate.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/v8/LaunchHelper.java (5 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/ISessionListener.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/ITestRunSession.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/Session.java (7 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/SessionsManager.java (5 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/TestRoot.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/serialize/Serializer.java (7 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/serialize/TestRunHandler.java (12 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/IJUnitHelpContextIds.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/PluginUI.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages.properties (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/UIPreferencesConstants.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/commands/DebugTestOutlineAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/commands/RunTestOutlineAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/dialogs/CompareResultDialog.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/editor/ruler/TestMethodActionDelegate.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/editor/ruler/TestMethodMarker.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/labelProvider/TestSessionLabelProvider.java (4 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/CounterPanel.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/TestRunnerViewPart.java (31 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/TestViewer.java (15 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ActivateOnErrorAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/CopyFailureListAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/FailuresOnlyFilterAction.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/IgnoredOnlyFilterAction.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ImportTestRunSessionAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/OpenTestAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/OpenUnderTestMethodAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/RerunAction.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ScrollLockAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowNextFailureAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowPreviousFailureAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowTestHierarchyAction.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowTimeAction.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowWebStackTraceAction.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ToggleOrientationAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ToggleSortingAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/ClearAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/RunnerViewHistory.java (7 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/FailureViewer.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/actions/CompareResultsAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/actions/CopyTraceAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/testitemaction/DebugTestCaseAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/testitemaction/RunTestCaseAction.java (3 hunks)

Finishing Touches

  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

🔭 Outside diff range comments (4)
viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/RunnerViewHistory.java (1)

Line range hint 114-127: Потенциальная опасность приведения типов в getImageDescriptor.

Приведение Object element к (Session) без проверки может вызвать ClassCastException, если element не является экземпляром Session. Рекомендуется добавить проверку типа.

Предлагаемый фикс:

 public ImageDescriptor getImageDescriptor(Object element) {
-   var session = (Session) element;
+   if (!(element instanceof Session)) {
+     return null; // или обработать ошибку соответствующим образом
+   }
+   var session = (Session) element;
    var imageProvider = testRunnerViewPart.getImageProvider();
    // дальнейшая логика
 }
viewer/viewer.iml (1)

Line range hint 6-18: Удалите дублирующиеся записи в additionalJARContents!

В конфигурации присутствуют повторяющиеся записи для META-INF, plugin.xml, icons и plugin.properties. Необходимо оставить только одну копию каждой записи.

         <additionalJARContents>
           <entry source="$MODULE_DIR$/META-INF" dest="META-INF" />
           <entry source="$MODULE_DIR$/plugin.xml" dest="plugin.xml" />
           <entry source="$MODULE_DIR$/icons" dest="icons" />
           <entry source="$MODULE_DIR$/plugin.properties" dest="plugin.properties" />
-          <entry source="$MODULE_DIR$/META-INF" dest="META-INF" />
-          <entry source="$MODULE_DIR$/plugin.xml" dest="plugin.xml" />
-          <entry source="$MODULE_DIR$/icons" dest="icons" />
-          <entry source="$MODULE_DIR$/plugin.properties" dest="plugin.properties" />
-          <entry source="$MODULE_DIR$/META-INF" dest="META-INF" />
-          <entry source="$MODULE_DIR$/plugin.xml" dest="plugin.xml" />
-          <entry source="$MODULE_DIR$/icons" dest="icons" />
-          <entry source="$MODULE_DIR$/plugin.properties" dest="plugin.properties" />
         </additionalJARContents>
viewer/src/main/java/ru/biatech/edt/junit/model/serialize/Serializer.java (2)

Line range hint 86-141: Улучшить обработку прерывания потока.

В методе importTestRunSession поток может остаться активным при прерывании. Также отсутствует освобождение ресурсов.

   public Session importTestRunSession(String url, String defaultProjectName, IProgressMonitor monitor) throws InvocationTargetException, InterruptedException {
     monitor.beginTask(JUnitMessages.JUnitModel_importing_from_url, IProgressMonitor.UNKNOWN);
     final String trimmedUrl = url.trim().replaceAll("\r\n?|\n", "");
     final TestRunHandler handler = new TestRunHandler(monitor);
     handler.fDefaultProjectName = defaultProjectName;
 
     final CoreException[] exception = {null};
     final Session[] session = {null};
+    Thread importThread = null;
 
     try {
-      Thread importThread = new Thread("JUnit URL importer") {
+      importThread = new Thread("JUnit URL importer") {
         @Override
         public void run() {
           try {
             SAXParserFactory parserFactory = SAXParserFactory.newInstance();
             SAXParser parser = parserFactory.newSAXParser();
             parser.parse(trimmedUrl, handler);
             session[0] = handler.getTestRunSession();
           } catch (OperationCanceledException e) {
             // canceled
           } catch (ParserConfigurationException | SAXException e) {
             storeImportError(e);
           } catch (IOException e) {
             storeImportError(e);
           } catch (IllegalArgumentException e) {
             storeImportError(e);
           }
         }
 
         private void storeImportError(Exception e) {
           exception[0] = new CoreException(new org.eclipse.core.runtime.Status(IStatus.ERROR,
               TestViewerPlugin.getPluginId(), JUnitMessages.JUnitModel_could_not_import, e));
         }
       };
       importThread.start();
 
       while (session[0] == null && exception[0] == null && !monitor.isCanceled()) {
         try {
           Thread.sleep(100);
         } catch (InterruptedException e) {
           Thread.currentThread().interrupt();
           throw e;
         }
       }
+    } finally {
+      if (importThread != null && importThread.isAlive()) {
+        importThread.interrupt();
+        try {
+          importThread.join(1000);
+        } catch (InterruptedException e) {
+          Thread.currentThread().interrupt();
+        }
+      }
+    }
 
     if (session[0] == null) {
       if (exception[0] != null) {
         throw new InvocationTargetException(exception[0]);
       } else {
-        importThread.interrupt();
         throw new InterruptedException();
       }
     }

Line range hint 144-160: Отсутствует освобождение ресурсов SAX парсера.

В методе importIntoTestRunSession ресурсы SAX парсера не освобождаются должным образом.

   public void importIntoTestRunSession(File swapFile, Session session) throws CoreException {
+    SAXParser parser = null;
     try {
       TestViewerPlugin.log().debug("Обновление отчета о тестировании: " + swapFile.getAbsolutePath());
       SAXParserFactory parserFactory = SAXParserFactory.newInstance();
-      SAXParser parser = parserFactory.newSAXParser();
+      parser = parserFactory.newSAXParser();
       TestRunHandler handler = new TestRunHandler(session);
       parser.parse(swapFile, handler);
     } catch (ParserConfigurationException | SAXException e) {
       throwImportError(swapFile, e);
     } catch (IOException e) {
       throwImportError(swapFile, e);
     } catch (IllegalArgumentException e) {
       throwImportError(swapFile, e);
+    } finally {
+      if (parser != null) {
+        try {
+          parser.reset();
+        } catch (Exception e) {
+          TestViewerPlugin.log().logError("Ошибка при освобождении ресурсов парсера", e);
+        }
+      }
     }
   }
🧹 Nitpick comments (25)
viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ActivateOnErrorAction.java (1)

36-36: Хорошая реализация, но можно улучшить!

Использование Preferences.putShowOnErrorOnly упрощает код и делает его более поддерживаемым. Однако стоит рассмотреть возможность добавления документации к методу run().

Предлагаю добавить документацию:

  @Override
+ /**
+  * Сохраняет настройку отображения только ошибок
+  * и обновляет пользовательский интерфейс
+  */
  public void run() {
viewer/src/main/java/ru/biatech/edt/junit/ui/UIPreferencesConstants.java (1)

Line range hint 30-56: Рекомендация по улучшению документации констант.

Для улучшения поддержки кода, рекомендуется добавить более подробную документацию к константам и методам, включая:

  • Примеры использования
  • Возможные значения
  • Влияние на поведение системы

Пример улучшенной документации:

   /**
    * Boolean preference controlling whether newly launched JUnit tests should be shown in all
    * JUnit views (in all windows).
+   * 
+   * When enabled (true):
+   * - New test runs will appear in all open JUnit views across all windows
+   * - Useful for multi-monitor setups or split window configurations
+   * 
+   * When disabled (false):
+   * - Test runs only appear in the active JUnit view
+   * 
+   * Default value: false
    */
viewer/src/main/java/ru/biatech/edt/junit/model/TestCaseElement.java (1)

28-30: Правильное использование Lombok аннотаций!

Хорошее решение использовать Lombok для уменьшения шаблонного кода. Переименование поля также улучшает читаемость кода.

Рекомендуется добавить JavaDoc к полю ignored, чтобы документировать его назначение:

+  /**
+   * Флаг, указывающий, что тестовый случай игнорируется
+   */
   @Getter
   @Setter
   private boolean ignored;
viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/ClearAction.java (2)

38-41: Логика инициализации корректна!

Переменная enabled используется для хранения флага активности. Рекомендуется использовать более описательное имя на русском языке, например, доступноДляОчистки.

-    var enabled = false;
+    var доступноДляОчистки = false;
     var sessions = TestViewerPlugin.core().getSessionsManager().getSessions();
     for (var session : sessions) {
       if (!session.isRunning() && !session.isStarting()) {
-        enabled = true;
+        доступноДляОчистки = true;
         break;
       }
     }
-    setEnabled(enabled);
+    setEnabled(доступноДляОчистки);

56-59: Предлагаю улучшить читаемость метода getRunningSessions!

Метод корректно фильтрует сессии, но можно улучшить его читаемость:

  1. Переименовать метод на русский язык для соответствия общему стилю
  2. Использовать более понятные имена переменных
  3. Добавить документацию метода
-  private List<Session> getRunningSessions() {
-    var sessions = TestViewerPlugin.core().getSessionsManager().getSessions();
-    sessions.removeIf(testRunSession -> !testRunSession.isRunning() && !testRunSession.isStarting());
-    return sessions;
+  /**
+   * Получает список активных сессий тестирования
+   * @return список сессий в состоянии выполнения или запуска
+   */
+  private List<Session> получитьАктивныеСессии() {
+    var активныеСессии = TestViewerPlugin.core().getSessionsManager().getSessions();
+    активныеСессии.removeIf(сессия -> !сессия.isRunning() && !сессия.isStarting());
+    return активныеСессии;
   }
viewer/src/main/java/ru/biatech/edt/junit/ui/labelProvider/TestSessionLabelProvider.java (1)

167-169: Улучшение читаемости условия

Упрощение проверки режима отображения делает код более понятным. Однако стоит добавить константу или перечисление для режимов отображения вместо использования магического числа LAYOUT_HIERARCHICAL.

-    if (fLayoutMode != TestRunnerViewPart.LAYOUT_HIERARCHICAL && element instanceof TestCaseElement) {
+    if (element instanceof TestCaseElement && fLayoutMode != TestRunnerViewPart.LAYOUT_HIERARCHICAL) {

Также рекомендуется определить enum для режимов отображения:

public enum LayoutMode {
    HIERARCHICAL,
    FLAT
}
viewer/src/main/java/ru/biatech/edt/junit/model/TestSuiteElement.java (2)

68-68: Рассмотрите оптимизацию создания массива!

Текущая реализация создает промежуточный массив при каждом вызове метода. Для оптимизации производительности рассмотрите возможность кэширования результата, если метод вызывается часто.

-    TestElement[] children = this.children.toArray(TestElement[]::new);
+    List<TestElement> childrenList = this.children;
+    int size = childrenList.size();
+    if (size == 0) {
+        return getSuiteStatus();
+    }
+    TestStatus cumulated = childrenList.get(0).getStatus();
+    for (int i = 1; i < size; i++) {
+        TestStatus childStatus = childrenList.get(i).getStatus();
+        cumulated = TestStatus.combineStatus(cumulated, childStatus);
+    }

86-92: Рассмотрите упрощение логики обработки статусов!

Текущая реализация содержит множество вложенных условий, что затрудняет понимание и поддержку кода. Предлагаю рассмотреть возможность разделения логики на отдельные методы.

+    private boolean isFirstChildRunning(TestElement child, TestStatus childStatus) {
+        return child == children.get(0) && childStatus.isRunning();
+    }
+
+    private boolean isLastChildCompleted(TestElement child, TestStatus childStatus) {
+        return child == children.get(children.size() - 1) && childStatus.isDone();
+    }
viewer/src/main/java/ru/biatech/edt/junit/model/serialize/TestRunHandler.java (4)

73-74: Добавьте проверку параметра session!

Рекомендуется добавить проверку на null для параметра session в конструкторе.

 public TestRunHandler(Session session) {
+    if (session == null) {
+        throw new IllegalArgumentException("session must not be null");
+    }
     this.session = session;
 }

88-93: Улучшите обработку прогресса!

Текущая реализация может привести к частым обновлениям UI при малых значениях line. Рекомендуется увеличить интервал обновления.

-  if (line - 20 >= lastReportedLine) {
-    line -= line % 20;
+  if (line - 100 >= lastReportedLine) {
+    line -= line % 100;
     lastReportedLine = line;
     progressMonitor.subTask(NLS.bind(JUnitMessages.TestRunHandler_lines_read, line));
   }

193-196: Добавьте комментарий к проверке на null!

Добавьте комментарий, поясняющий логику fallback на testSuite при null testCase.

 TestElement testElement = testCase;
+// Если ошибка произошла на уровне suite, используем его как контейнер ошибки
 if (testElement == null)
   testElement = testSuite;

Line range hint 299-320: Рассмотрите выделение логики работы с проектом!

Логика получения проекта повторяется в нескольких местах. Рекомендуется выделить её в отдельный метод.

+  private IV8Project resolveProject(String projectName) {
+    if (Strings.isNullOrEmpty(projectName)) {
+      projectName = fDefaultProjectName;
+    }
+    return !Strings.isNullOrEmpty(projectName) ? Projects.getProject(projectName) : null;
+  }

   Session readSession(Attributes attributes) {
     String name = attributes.getValue(IXMLTags.ATTR_NAME);
     String projectName = attributes.getValue(IXMLTags.ATTR_PROJECT);
-    IV8Project project = null;
-
-    if (Strings.isNullOrEmpty(projectName)) {
-      projectName = fDefaultProjectName;
-    }
-    if (!Strings.isNullOrEmpty(projectName)) {
-      project = Projects.getProject(projectName);
-    }
+    IV8Project project = resolveProject(projectName);
viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/RunnerViewHistory.java (1)

140-146: Неправильное форматирование даты в getText.

Метод DateFormat.getDateTimeInstance() может использовать локаль по умолчанию, что может привести к несоответствиям в отображении даты и времени. Рекомендуется указать конкретную локаль или формат.

Предлагаемый фикс:

 var startTime = DateFormat.getDateTimeInstance().format(new Date(session.getStartTime()));
+ // Например, использовать локаль
+ var startTime = DateFormat.getDateTimeInstance(DateFormat.SHORT, DateFormat.SHORT, Locale.getDefault()).format(new Date(session.getStartTime()));
viewer/src/main/java/ru/biatech/edt/junit/model/SessionsManager.java (1)

Line range hint 232-294: Удалите закомментированный код для улучшения читаемости

В данном блоке присутствуют закомментированные участки кода. Если они больше не требуются, рекомендуется удалить их, чтобы повысить читаемость и поддерживаемость кода.

viewer/src/main/java/ru/biatech/edt/junit/ui/report/TestRunnerViewPart.java (3)

685-685: Обновите вызов метода для соответствия новым именованиям

Метод addTestRunSessionListener в SessionsManager может быть переименован в addSessionListener для согласованности с обновленными именами классов и методов.

Примените следующий дифф для исправления:

-   TestViewerPlugin.core().getSessionsManager().addTestRunSessionListener(fTestRunSessionListener);
+   TestViewerPlugin.core().getSessionsManager().addSessionListener(fTestRunSessionListener);

957-971: Проверьте использование устаревших методов и переменных

В методе sessionAdded используются переменные и методы с устаревшими именами, такими как getTestRunName(). Рекомендуется обновить их в соответствии с новыми названиями для предотвращения путаницы.


1069-1070: Оптимизируйте форматирование строк

Вместо использования MessageFormat.format для форматирования строк, можно использовать String.format, что обеспечит более простое и быстрое форматирование, если интернационализация не требуется.

Примените следующий дифф для исправления:

-   String msg = MessageFormat.format(JUnitMessages.TestRunnerViewPart_message_started, className, method);
+   String msg = String.format(JUnitMessages.TestRunnerViewPart_message_started, className, method);
viewer/src/main/java/ru/biatech/edt/junit/Preferences.java (2)

28-31: Добавьте документацию к методу!

Метод корректно реализован, но рекомендуется добавить JavaDoc с описанием параметра и назначения метода.


37-39: Сделайте максимальное количество тестовых запусков настраиваемым!

Рекомендуется вынести значение 10 в настраиваемую константу или параметр конфигурации, чтобы пользователи могли изменять его при необходимости.

-  public int getMaxTestRuns() {
-    return 10;
-  }
+  public static final String MAX_TEST_RUNS = Constants.PLUGIN_ID + ".max_test_runs";
+  private static final int DEFAULT_MAX_TEST_RUNS = 10;
+
+  public int getMaxTestRuns() {
+    return Platform.getPreferencesService()
+        .getInt(TestViewerPlugin.getPluginId(), MAX_TEST_RUNS, DEFAULT_MAX_TEST_RUNS, null);
+  }
viewer/src/main/java/ru/biatech/edt/junit/BasicElementLabels.java (1)

64-66: Обновите JavaDoc после переименования метода!

Переименование метода логично, но необходимо обновить документацию, убрав упоминание "Java" из описания, так как метод теперь используется для элементов в целом.

   /**
-   * Returns a label for Java element name. Example is 'new Test<? extends List>() { ...}'.
-   * This method should only be used for simple element names. Use
-   * JavaElementLabels to create a label from a Java element.
+   * Returns a label for element name. Example is 'new Test<? extends List>() { ...}'.
+   * This method should only be used for simple element names.
    *
-   * @param name the Java element name.
+   * @param name the element name.
    * @return the label for the Java element
    */
viewer/src/main/java/ru/biatech/edt/junit/Core.java (2)

33-39: Класс реализован как синглтон без соответствующего паттерна.

Рекомендуется реализовать паттерн Singleton для обеспечения единственности экземпляра класса, так как комментарий указывает на это намерение.

 public class Core {
+  private static volatile Core instance;
+  
+  public static Core getInstance() {
+    if (instance == null) {
+      synchronized (Core.class) {
+        if (instance == null) {
+          instance = new Core();
+        }
+      }
+    }
+    return instance;
+  }
+
+  private Core() {
     @Getter
     private final SessionsManager sessionsManager = new SessionsManager();
   }

48-56: Метод помечен как устаревший без указания альтернативы.

В документации к устаревшему методу следует указать альтернативный способ регистрации слушателей.

   /**
    * Adds a TestRun listener to the collection of listeners
    *
    * @param newListener the listener to add
-   * @deprecated to avoid deprecation warnings
+   * @deprecated Используйте SessionsManager#addListener(TestRunListener) вместо этого метода
    */
viewer/src/main/java/ru/biatech/edt/junit/kinds/TestKindRegistry.java (1)

Line range hint 41-50: Улучшить реализацию синглтона для потокобезопасности.

Текущая реализация синглтона не является потокобезопасной. Рекомендуется использовать double-checked locking или holder pattern.

-  private static TestKindRegistry fgRegistry;
+  private static class Holder {
+    static final TestKindRegistry INSTANCE = new TestKindRegistry(
+      Platform.getExtensionRegistry().getExtensionPoint(Constants.ID_EXTENSION_POINT_TEST_KINDS)
+    );
+  }
 
   public static TestKindRegistry getDefault() {
-    if (fgRegistry != null) return fgRegistry;
-    fgRegistry = new TestKindRegistry(Platform.getExtensionRegistry().getExtensionPoint(Constants.ID_EXTENSION_POINT_TEST_KINDS));
-    return fgRegistry;
+    return Holder.INSTANCE;
   }
viewer/src/main/java/ru/biatech/edt/junit/TestViewerPlugin.java (1)

135-142: Улучшить очистку ресурсов при остановке плагина.

В методе stop следует обеспечить корректное освобождение всех ресурсов.

   @Override
   public void stop(BundleContext bundleContext) throws Exception {
-    LifecycleMonitor.stop();
-    core().getSessionsManager().stop();
-    plugin = null;
-    super.stop(bundleContext);
+    try {
+      LifecycleMonitor.stop();
+    } finally {
+      try {
+        if (core != null) {
+          core.getSessionsManager().stop();
+        }
+      } finally {
+        plugin = null;
+        super.stop(bundleContext);
+      }
+    }
   }
viewer/plugin_ru.properties (1)

18-34: Корректные переводы, но рекомендуется убрать восклицательные знаки.

Переводы точные и понятные, но рекомендуется убрать восклицательные знаки из следующих строк для соответствия общему стилю интерфейса:

-JUnitShortcut.description.debug = \u041E\u0442\u043B\u0430\u0434\u043A\u0430 \u043C\u043E\u0434\u0443\u043B\u044C\u043D\u043E\u0433\u043E \u0442\u0435\u0441\u0442\u0430!
+JUnitShortcut.description.debug = \u041E\u0442\u043B\u0430\u0434\u043A\u0430 \u043C\u043E\u0434\u0443\u043B\u044C\u043D\u043E\u0433\u043E \u0442\u0435\u0441\u0442\u0430
-JUnitShortcut.description.run   = \u0417\u0430\u043F\u0443\u0441\u0442\u0438\u0442\u044C \u043C\u043E\u0434\u0443\u043B\u044C\u043D\u044B\u0439 \u0442\u0435\u0441\u0442!
+JUnitShortcut.description.run   = \u0417\u0430\u043F\u0443\u0441\u0442\u0438\u0442\u044C \u043C\u043E\u0434\u0443\u043B\u044C\u043D\u044B\u0439 \u0442\u0435\u0441\u0442
-JUnitShortcut.label             = \u041C\u043E\u0434\u0443\u043B\u044C\u043D\u044B\u0439 \u0442\u0435\u0441\u0442!
+JUnitShortcut.label             = \u041C\u043E\u0434\u0443\u043B\u044C\u043D\u044B\u0439 \u0442\u0435\u0441\u0442
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 792d65d and ee3f120.

📒 Files selected for processing (43)
  • viewer/META-INF/MANIFEST.MF (0 hunks)
  • viewer/plugin.properties (1 hunks)
  • viewer/plugin.xml (0 hunks)
  • viewer/plugin_ru.properties (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/BasicElementLabels.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/Constants.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/Core.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/JUnitCore.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/JUnitPreferencesConstants.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/JunitPreferenceInitializer.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/Logger.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/Preferences.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/TestRunListener.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/TestViewerPlugin.java (4 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/kinds/TestKindRegistry.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/lifecycle/LifecycleMonitor.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/v8/RerunHelper.java (6 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/ITestElement.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/ITestRunSessionListener.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/MessageIds.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/Session.java (5 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/SessionsManager.java (6 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/TestCaseElement.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/TestElement.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/TestRoot.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/TestRunListenerAdapter.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/TestSuiteElement.java (5 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/serialize/Serializer.java (5 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/serialize/TestRunHandler.java (12 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/JUnitMessages.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/JUnitMessages.properties (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/UIPreferencesConstants.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/labelProvider/TestSessionLabelProvider.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/TestRunnerViewPart.java (22 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/TestViewer.java (12 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ActivateOnErrorAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ImportTestRunSessionAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/RerunLastAction.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/RerunLastFailedFirstAction.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/ClearAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/RunnerViewHistory.java (5 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/ViewHistory.java (0 hunks)
  • viewer/viewer.iml (1 hunks)
💤 Files with no reviewable changes (15)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/lifecycle/LifecycleMonitor.java
  • viewer/src/main/java/ru/biatech/edt/junit/model/MessageIds.java
  • viewer/src/main/java/ru/biatech/edt/junit/JUnitPreferencesConstants.java
  • viewer/META-INF/MANIFEST.MF
  • viewer/src/main/java/ru/biatech/edt/junit/JUnitCore.java
  • viewer/src/main/java/ru/biatech/edt/junit/JunitPreferenceInitializer.java
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/RerunLastFailedFirstAction.java
  • viewer/src/main/java/ru/biatech/edt/junit/ui/JUnitMessages.java
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/RerunLastAction.java
  • viewer/src/main/java/ru/biatech/edt/junit/model/TestRoot.java
  • viewer/src/main/java/ru/biatech/edt/junit/ui/JUnitMessages.properties
  • viewer/src/main/java/ru/biatech/edt/junit/model/ITestElement.java
  • viewer/src/main/java/ru/biatech/edt/junit/model/TestElement.java
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/ViewHistory.java
  • viewer/plugin.xml
🔇 Additional comments (32)
viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ActivateOnErrorAction.java (1)

20-20: Одобряю переход на централизованное управление настройками!

Замена прямого использования InstanceScope на специализированный класс Preferences улучшает инкапсуляцию логики управления настройками.

viewer/src/main/java/ru/biatech/edt/junit/ui/UIPreferencesConstants.java (2)

30-30: Переименование класса соответствует принципам рефакторинга.

Упрощение имени класса делает код более читаемым, при этом сохраняя его функциональность и область ответственности.


39-41: Конструктор корректно обновлен.

Приватный конструктор правильно переименован в соответствии с новым именем класса.

viewer/src/main/java/ru/biatech/edt/junit/model/TestCaseElement.java (2)

21-23: Корректное добавление импортов Lombok!

Импорты Lombok аннотаций добавлены правильно и поддерживают рефакторинг кода.


66-70: Корректное обновление метода getTestResult!

Метод правильно обновлен для использования нового поля ignored. Логика работы сохранена без изменений.

viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/ClearAction.java (2)

21-21: Корректное обновление импорта!

Замена импорта TestRunSession на Session соответствует новой архитектуре управления сеансами тестирования.


51-53: Метод run реализован корректно!

Логика получения и установки активных сессий реализована правильно, с корректной обработкой пустого списка.

viewer/src/main/java/ru/biatech/edt/junit/ui/labelProvider/TestSessionLabelProvider.java (2)

151-151: Упрощение логики получения имён элементов

Изменение метода получения имён элементов с getJavaElementName на getElementName выглядит логичным упрощением, так как теперь метод более универсален и не привязан к Java-специфичной логике.

Also applies to: 155-155


106-106: Проверьте корректность отображения имён после рефакторинга

Замена getJavaElementName на getElementName может повлиять на форматирование отображаемых имён тестов. Убедитесь, что имена тестов корректно отображаются для всех поддерживаемых типов тестов.

✅ Verification successful

Изменение метода форматирования имён проверено

Анализ кодовой базы показывает, что замена getJavaElementName на getElementName является частью последовательного рефакторинга, и метод BasicElementLabels.getElementName корректно используется во всех контекстах отображения имён тестов, классов и методов.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Поиск всех мест использования BasicElementLabels.getElementName
rg "BasicElementLabels\.getElementName" -A 2

Length of output: 3673

viewer/src/main/java/ru/biatech/edt/junit/model/TestSuiteElement.java (4)

26-26: Улучшение именования поля и инициализации!

Переименование поля с fChildren на children соответствует современным соглашениям об именовании в Java. Прямая инициализация ArrayList является более чистым подходом.


49-49: Отличное использование современного синтаксиса Java!

Использование ссылки на метод (ITestElement[]::new) вместо явного создания массива делает код более лаконичным и читаемым.


53-53: Корректное обновление ссылки на поле!

Метод правильно обновлен для использования нового поля children.


146-146: Корректное обновление метода toString!

Метод правильно обновлен для использования нового поля children.

viewer/src/main/java/ru/biatech/edt/junit/model/TestRunListenerAdapter.java (2)

29-32: Улучшение именования и инициализации поля session!

Изменения соответствуют современным конвенциям именования в Java. Использование this.session в конструкторе предотвращает затенение параметра.


41-41: Корректное обновление методов уведомления слушателей!

Методы fireSessionStarted и fireSessionFinished корректно обновлены для использования переименованного поля session.

Also applies to: 47-47

viewer/src/main/java/ru/biatech/edt/junit/model/serialize/TestRunHandler.java (3)

35-35: Корректное добавление импорта!

Добавление импорта Session соответствует рефакторингу и замене TestRunSession.


58-64: Улучшение именования полей класса!

Переименование полей улучшает читаемость кода:

  • Удалены префиксы 'f'
  • Имена стали более понятными и соответствуют общепринятым конвенциям

339-342: Проверьте порядок обновления состояния!

Убедитесь, что порядок обновления состояния (status = null) после регистрации ошибки не приведет к проблемам при параллельном выполнении.

✅ Verification successful

Порядок обновления состояния безопасен

Анализ показывает, что TestRunHandler создается для каждой операции разбора отдельно и не используется совместно между потоками. Обнуление testStatus после регистрации ошибки безопасно, так как происходит в контексте одного потока обработки.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Поиск мест использования testStatus в многопоточном контексте
rg -l "extends Thread|implements Runnable|ExecutorService|CompletableFuture" | xargs rg "testStatus"

Length of output: 888


Script:

#!/bin/bash
# Find TestRunHandler instantiation and usage
echo "=== TestRunHandler class definition ==="
ast-grep --pattern 'class TestRunHandler { $$$ }'

echo -e "\n=== TestRunHandler instantiation ==="
rg "new TestRunHandler" -A 2

echo -e "\n=== Synchronization patterns ==="
rg "synchronized|volatile|Lock|Atomic" viewer/src/main/java/ru/biatech/edt/junit/model/serialize/TestRunHandler.java

Length of output: 1523

viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/RunnerViewHistory.java (2)

27-27: Отсутствуют необходимые импорты для класса Session.

Вы добавили импорт для ru.biatech.edt.junit.model.Session, однако убедитесь, что все связанные классы импортированы и актуальны после изменения с TestRunSession на Session.


41-41: Проверить совместимость с обобщенным типом ViewHistory<Session>.

Изменение обобщенного типа на Session может повлиять на поведение методов базового класса ViewHistory. Убедитесь, что все методы работают корректно с новым типом.

viewer/src/main/java/ru/biatech/edt/junit/model/SessionsManager.java (1)

168-173: Проверьте корректность удаления старых сессий

В методе addSession при удалении старых сессий рекомендуется убедиться, что удаляются только завершенные и неиспользуемые сессии. Это предотвратит возможную утечку памяти и обеспечит корректное управление сессиями.

Вы можете выполнить следующий скрипт для проверки текущих сессий:

viewer/src/main/java/ru/biatech/edt/junit/Constants.java (1)

21-27: Корректная реализация класса констант!

Класс грамотно реализован с использованием аннотации @UtilityClass и содержит все необходимые константы для идентификации плагина и точек расширения.

viewer/viewer.iml (1)

24-36: Корректная конфигурация модуля!

Конфигурация корректно определяет структуру исходных файлов, ресурсов и зависимостей модуля.

viewer/src/main/java/ru/biatech/edt/junit/Logger.java (1)

29-29: Перенос константы в централизованный класс - одобрено!

Перемещение идентификатора плагина в класс Constants улучшает организацию кода и соответствует принципу единой ответственности.

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ImportTestRunSessionAction.java (1)

25-25: Обновление импорта и вызова метода в соответствии с рефакторингом

Изменения корректно отражают переход от JUnitModel к SessionsManager, что делает код более понятным и соответствующим текущей архитектуре.

Also applies to: 60-60

viewer/src/main/java/ru/biatech/edt/junit/launcher/v8/RerunHelper.java (1)

24-24: Обновление типа Session во всех методах класса

Последовательное обновление типа с TestRunSession на Session во всех методах класса. Изменения являются частью общего рефакторинга и не затрагивают функциональность.

Also applies to: 38-38, 48-48, 63-63, 80-80, 94-94

viewer/src/main/java/ru/biatech/edt/junit/TestRunListener.java (1)

23-23: Обновление документации в соответствии с рефакторингом

Документация корректно обновлена для отражения перехода от JUnitCore к Core, что поддерживает актуальность документации.

viewer/src/main/java/ru/biatech/edt/junit/model/Session.java (2)

155-160: Корректная реализация сброса состояния.

Метод правильно обнуляет все счетчики и создает новый корневой элемент теста.


58-83: Проверьте потокобезопасность после удаления AtomicInteger.

Замена AtomicInteger на примитивный тип int может привести к проблемам синхронизации при параллельном доступе к счетчикам. Убедитесь, что доступ к этим полям правильно синхронизирован.

✅ Verification successful

Удаление AtomicInteger безопасно в данном контексте.

Анализ показал, что класс Session используется только в контексте UI-потока Eclipse RCP, где все модификации счетчиков происходят последовательно при обработке событий тестирования. Синхронизация через AtomicInteger не требуется, так как отсутствует параллельный доступ к полям.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Поиск мест, где изменяются счетчики
rg -A 2 "startedCount|ignoredCount|assumptionFailureCount|errorCount|failureCount|totalCount"

Length of output: 11284


Script:

#!/bin/bash
# Check for thread-related code
rg -i "thread|executor|concurrent|async" viewer/src/main/java/ru/biatech/edt/junit/
# Check for thread safety annotations
rg -i "@threadsafe|@notsafe|@guarded" viewer/src/main/java/ru/biatech/edt/junit/

Length of output: 3852

viewer/src/main/java/ru/biatech/edt/junit/ui/report/TestViewer.java (2)

Line range hint 52-103: Корректное переименование в соответствии с рефакторингом.

Переименование поля fTestRunSession в session и обновление импортов соответствует общему направлению рефакторинга и улучшает читаемость кода.


Line range hint 799-810: Корректное обновление ссылок на сессию в фильтрах.

Обновление использования session в классах FailuresOnlyFilter и IgnoredOnlyFilter выполнено последовательно и не меняет функциональность.

viewer/plugin.properties (1)

20-36: Корректное добавление и обновление свойств для YAxUnit.

Новые свойства имеют понятные описания и соответствуют общей концепции интеграции YAxUnit.

@@ -69,18 +69,18 @@ public void sessionStarted() {
@Override
public void sessionEnded(long elapsedTime) {
fireSessionFinished();
fSession.swapOut();
session.swapOut();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Обнаружена критическая проблема в реализации метода swapOut!

Метод swapOut() в классе Session полностью пустой, хотя должен отвечать за оптимизацию памяти. Весь код закомментирован, включая важные проверки состояния сессии и корневого теста. Это означает, что:

  • Оптимизация памяти не работает
  • Отсутствует обработка ошибок
  • Нет проверок состояния сессии перед выгрузкой

Требуется:

  • Реализовать логику выгрузки данных сессии
  • Добавить проверки состояния
  • Реализовать обработку ошибок
  • Решить вопрос с TODO комментарием "Не ясно, нужно или нет"
🔗 Analysis chain

Проверьте реализацию метода swapOut!

Методы sessionEnded, sessionStopped и sessionTerminated вызывают session.swapOut() для оптимизации памяти. Рекомендуется убедиться, что:

  • Сессия корректно сохраняется перед выгрузкой
  • Обработка ошибок при сериализации реализована должным образом

Also applies to: 78-78, 83-83

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Поиск реализации метода swapOut и проверка обработки ошибок
ast-grep --pattern 'class Session {
  $$$
  swapOut() {
    $$$
  }
  $$$
}'

Length of output: 78


Script:

#!/bin/bash
# Найдем файлы, содержащие определение класса Session
rg "class Session" -l

# Поищем все упоминания метода swapOut
rg "swapOut" -A 5

Length of output: 4656


TestViewerPlugin.ui().asyncShowTestRunnerViewPart();

Files.deleteIfExists(reportPath);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Избегайте удаления файла отчета без проверки необходимости

Удаление файла отчета сразу после его загрузки может привести к потере данных, если файл потребуется для дальнейшей отладки или анализа. Рекомендуется убедиться, что файл больше не понадобится, прежде чем удалять его.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🔭 Outside diff range comments (1)
viewer/src/main/java/ru/biatech/edt/junit/launcher/ui/LaunchControl.java (1)

Line range hint 74-74: Необходимо вынести текст в ресурсы

Строка "Выводить лог в консоль" должна быть перемещена в UIMessages для обеспечения единообразия локализации интерфейса.

Предлагаемые изменения:

  1. Добавьте новую константу в UIMessages:
public static String LaunchConfigurationTab_ShowConsoleLog;
  1. Измените код:
-    appendLabel(grpSettings, "Выводить лог в консоль");
+    appendLabel(grpSettings, UIMessages.LaunchConfigurationTab_ShowConsoleLog);
♻️ Duplicate comments (1)
viewer/src/main/java/ru/biatech/edt/junit/ui/report/TestRunnerViewPart.java (1)

299-307: 🛠️ Refactor suggestion

Переименуйте методы для согласованности

Методы все еще содержат старое название TestRunSession. Необходимо переименовать для поддержания согласованности.

Примените следующий дифф:

-  public Session getTestRunSession() {
+  public Session getSession() {
     return session;
   }

-  public Session setActiveTestRunSession(Session session) {
+  public Session setActiveSession(Session session) {
     // тело метода
   }
🧹 Nitpick comments (8)
viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/OpenTestAction.java (1)

59-59: Корректное обновление сообщения об ошибке

Использование UIMessages для сообщений об ошибках соответствует новой структуре обработки сообщений.

Рекомендуется добавить константу для повторяющегося использования UIMessages.OpenTestAction_label:

public class OpenTestAction extends OpenEditorAction {
+  private static final String ACTION_LABEL = UIMessages.OpenTestAction_label;

  public OpenTestAction(TestRunnerViewPart testRunnerPart, TestCaseElement testCase) {
    this(testRunnerPart, testCase.getClassName());
  }

  public OpenTestAction(TestRunnerViewPart testRunnerPart, String testClassName) {
-    super(UIMessages.OpenTestAction_label, ImageProvider.ACTION_GOTO_TESTSUITE, testRunnerPart, testClassName);
+    super(ACTION_LABEL, ImageProvider.ACTION_GOTO_TESTSUITE, testRunnerPart, testClassName);
  }

  @Override
  protected MethodReference getPosition() {
    // ...
    if (ref == null) {
-      Dialogs.showError(UIMessages.OpenTestAction_label, UIMessages.OpenTestAction_error_not_found);
+      Dialogs.showError(ACTION_LABEL, UIMessages.OpenTestAction_error_not_found);
    }
    return ref;
  }
}
viewer/src/main/java/ru/biatech/edt/junit/ui/editor/ruler/TestMethodMarker.java (1)

74-74: Корректная замена форматирования сообщения!

Использование UIMessages.TestMethodMarker_LaunchTest правильно интегрируется с MessageFormat.

Предлагаю исправить опечатку в названии метода: createMarketcreateMarker, так как метод создает маркер, а не рынок.

-  private void createMarket(IResource resource, Method method) {
+  private void createMarker(IResource resource, Method method) {
viewer/src/main/java/ru/biatech/edt/junit/ui/report/CounterPanel.java (1)

88-98: Корректная замена форматирования сообщений!

Все шаблоны сообщений успешно обновлены для использования UIMessages. Логика обработки различных состояний тестов (пропущенные, неудачные проверки предусловий) сохранена корректно.

Рекомендуется добавить комментарии к строковым шаблонам для упрощения будущей поддержки.

viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/actions/CopyTraceAction.java (1)

72-72: Предложение по улучшению читаемости кода.

Рекомендуется разбить длинную строку на несколько строк для улучшения читаемости:

-      if (MessageDialog.openQuestion(TestViewerPlugin.ui().getShell(), UIMessages.CopyTraceAction_problem, UIMessages.CopyTraceAction_clipboard_busy))
+      if (MessageDialog.openQuestion(
+          TestViewerPlugin.ui().getShell(),
+          UIMessages.CopyTraceAction_problem,
+          UIMessages.CopyTraceAction_clipboard_busy))
viewer/src/main/java/ru/biatech/edt/junit/ui/PluginUI.java (2)

Line range hint 89-90: Удалите пустой блок else.

Пустой блок else не несет никакой функциональной нагрузки и может вводить в заблуждение при чтении кода.

Примените следующие изменения:

      } else {
      }

Line range hint 91-93: Улучшите обработку ошибок при инициализации представления.

При возникновении ошибки инициализации представления, пользователь не получает никакого уведомления. Рекомендуется добавить отображение сообщения об ошибке.

Предлагаю обновить обработку исключения:

    } catch (PartInitException pie) {
      TestViewerPlugin.log().logError(pie);
+     MessageDialog.openError(getShell(), "Ошибка", "Не удалось открыть представление тестов: " + pie.getMessage());
    }
viewer/src/main/java/ru/biatech/edt/junit/ui/UIPreferencesConstants.java (1)

Line range hint 43-55: Рекомендуется добавить документацию к методам

Методы getShowInAllViews и setShowInAllViews следует дополнить javadoc-комментариями для улучшения читаемости кода.

Предлагаемые изменения:

+  /**
+   * Получает настройку отображения тестов во всех окнах
+   * @return true если тесты должны отображаться во всех окнах
+   */
   public static boolean getShowInAllViews() {
     return Platform.getPreferencesService().getBoolean(PluginUI.PLUGIN_ID, SHOW_IN_ALL_VIEWS, SHOW_IN_ALL_VIEWS_DEFAULT, null);
   }

+  /**
+   * Устанавливает настройку отображения тестов во всех окнах
+   * @param show true для отображения тестов во всех окнах
+   */
   public static void setShowInAllViews(boolean show) {
viewer/src/main/java/ru/biatech/edt/junit/model/SessionsManager.java (1)

Line range hint 168-192: Оптимизируйте синхронизацию

Текущий synchronized блок слишком большой и может вызвать проблемы с производительностью при большом количестве сессий.

Рекомендуется:

  1. Уменьшить размер synchronized блока
  2. Использовать CopyOnWriteArrayList для безопасной итерации
   public void addSession(@NonNull Session session) {
-    var toRemove = new ArrayList<Session>();
+    List<Session> toRemove;
 
     synchronized (this) {
       Assert.isLegal(!sessions.contains(session));
       sessions.addFirst(session);
 
       var maxCount = Preferences.getMaxTestRuns();
       var size = sessions.size();
-      if (size > maxCount) {
-        var excess = sessions.subList(maxCount, size);
-        for (var iter = excess.iterator(); iter.hasNext(); ) {
-          var oldSession = iter.next();
-          if (!oldSession.isStarting() && !oldSession.isRunning() && !oldSession.isKeptAlive()) {
-            toRemove.add(oldSession);
-            iter.remove();
-          }
-        }
-      }
+      toRemove = getSessionsToRemove(maxCount, size);
     }
 
     for (var oldSession : toRemove) {
       notifySessionRemoved(oldSession);
     }
     notifySessionAdded(session);
   }
 
+  private synchronized List<Session> getSessionsToRemove(int maxCount, int size) {
+    if (size <= maxCount) {
+      return Collections.emptyList();
+    }
+    var toRemove = new ArrayList<Session>();
+    var excess = sessions.subList(maxCount, size);
+    for (var iter = excess.iterator(); iter.hasNext(); ) {
+      var oldSession = iter.next();
+      if (!oldSession.isStarting() && !oldSession.isRunning() && !oldSession.isKeptAlive()) {
+        toRemove.add(oldSession);
+        iter.remove();
+      }
+    }
+    return toRemove;
+  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee3f120 and 964e769.

📒 Files selected for processing (47)
  • viewer/src/main/java/ru/biatech/edt/junit/BasicElementLabels.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/TestViewerPlugin.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/lifecycle/LifecycleMonitor.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/ui/LaunchConfigurationTab.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/ui/LaunchControl.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/v8/LaunchConfigurationDelegate.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/v8/LaunchHelper.java (5 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/SessionsManager.java (5 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/serialize/Serializer.java (7 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/model/serialize/TestRunHandler.java (12 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/IJUnitHelpContextIds.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/PluginUI.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages.properties (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/UIPreferencesConstants.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/commands/DebugTestOutlineAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/commands/RunTestOutlineAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/dialogs/CompareResultDialog.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/editor/ruler/TestMethodActionDelegate.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/editor/ruler/TestMethodMarker.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/labelProvider/TestSessionLabelProvider.java (4 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/CounterPanel.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/TestRunnerViewPart.java (31 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/TestViewer.java (15 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ActivateOnErrorAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/CopyFailureListAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/FailuresOnlyFilterAction.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/IgnoredOnlyFilterAction.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ImportTestRunSessionAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/OpenTestAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/OpenUnderTestMethodAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/RerunAction.java (0 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ScrollLockAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowNextFailureAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowPreviousFailureAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowTestHierarchyAction.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowTimeAction.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowWebStackTraceAction.java (1 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ToggleOrientationAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ToggleSortingAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/ClearAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/RunnerViewHistory.java (7 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/FailureViewer.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/actions/CompareResultsAction.java (2 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/actions/CopyTraceAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/testitemaction/DebugTestCaseAction.java (3 hunks)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/testitemaction/RunTestCaseAction.java (3 hunks)
💤 Files with no reviewable changes (3)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/RerunAction.java
  • viewer/src/main/java/ru/biatech/edt/junit/ui/IJUnitHelpContextIds.java
  • viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages.properties
✅ Files skipped from review due to trivial changes (1)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/v8/LaunchConfigurationDelegate.java
🚧 Files skipped from review as they are similar to previous changes (3)
  • viewer/src/main/java/ru/biatech/edt/junit/launcher/lifecycle/LifecycleMonitor.java
  • viewer/src/main/java/ru/biatech/edt/junit/ui/labelProvider/TestSessionLabelProvider.java
  • viewer/src/main/java/ru/biatech/edt/junit/BasicElementLabels.java
👮 Files not reviewed due to content moderation or server errors (5)
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ActivateOnErrorAction.java
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/ClearAction.java
  • viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ImportTestRunSessionAction.java
  • viewer/src/main/java/ru/biatech/edt/junit/model/serialize/Serializer.java
  • viewer/src/main/java/ru/biatech/edt/junit/TestViewerPlugin.java
🔇 Additional comments (75)
viewer/src/main/java/ru/biatech/edt/junit/ui/commands/DebugTestOutlineAction.java (2)

22-22: Корректное обновление импорта!

Замена JUnitMessages на UIMessages соответствует общей стратегии рефакторинга по консолидации сообщений пользовательского интерфейса.


37-37: Корректное обновление сообщений об ошибках!

Использование UIMessages вместо JUnitMessages обеспечивает согласованность с обновленной структурой сообщений.

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ScrollLockAction.java (2)

20-20: Корректное обновление импортов

Замена JUnitMessages на UIMessages соответствует общему рефакторингу системы сообщений в приложении.


29-30: Корректное обновление конструктора

Изменения соответствуют цели PR по удалению неиспользуемого кода:

  • Обновлены ссылки на сообщения для использования UIMessages
  • Удалена неиспользуемая интеграция с системой помощи
viewer/src/main/java/ru/biatech/edt/junit/ui/commands/RunTestOutlineAction.java (2)

22-22: Корректное обновление импорта!

Замена JUnitMessages на UIMessages соответствует общему рефакторингу системы сообщений в проекте.


37-37: Корректное обновление ссылок на сообщения!

Использование констант из UIMessages вместо JUnitMessages согласуется с обновлением системы сообщений. Функциональность осталась прежней.

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ToggleSortingAction.java (2)

20-20: Корректное изменение импорта!

Замена JUnitMessages на UIMessages соответствует общей стратегии рефакторинга обработки сообщений в приложении.


30-36: Последовательное обновление ссылок на сообщения!

Все ссылки на сообщения корректно обновлены для использования UIMessages вместо JUnitMessages. Изменения последовательны и не влияют на функциональность.

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/IgnoredOnlyFilterAction.java (2)

20-20: Корректная замена импорта!

Замена JUnitMessages на UIMessages является частью рефакторинга для централизации UI сообщений.


26-27: Корректное обновление ссылок на сообщения!

Обновление ссылок на UIMessages соответствует изменению импорта и сохраняет существующую функциональность.

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/OpenUnderTestMethodAction.java (2)

22-22: Корректная замена JUnitMessages на UIMessages

Рефакторинг обработки сообщений выполнен последовательно. Все ссылки на JUnitMessages заменены на UIMessages, что соответствует общей стратегии реорганизации кодовой базы.

Also applies to: 33-33, 43-43


Line range hint 29-44: Хорошая структура и реализация класса

Класс имеет чёткую единственную ответственность и хорошо организованную логику:

  • Корректное наследование от OpenEditorAction
  • Чистое разделение ответственности между TestKindRegistry, TestsManager и ReferencedMethodHelper
  • Правильная обработка ошибок
viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/actions/CompareResultsAction.java (2)

23-23: Корректная замена импорта!

Замена JUnitMessages на UIMessages соответствует общей стратегии рефакторинга в проекте.


36-39: Корректное обновление конструктора!

Обновление ссылок на сообщения с использованием UIMessages выполнено последовательно и согласованно. Функциональность сохранена при переходе на новую систему сообщений.

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/OpenTestAction.java (2)

22-22: Корректное добавление импорта UIMessages

Замена JUnitMessages на UIMessages соответствует общей реструктуризации обработки сообщений в приложении.


41-41: Правильное использование UIMessages в конструкторе

Обновление конструктора для использования UIMessages.OpenTestAction_label вместо JUnitMessages поддерживает единообразие в коде.

viewer/src/main/java/ru/biatech/edt/junit/launcher/ui/LaunchControl.java (1)

33-33: Корректное изменение импорта!

Замена импорта с JUnitMessages на UIMessages соответствует общей стратегии рефакторинга.

viewer/src/main/java/ru/biatech/edt/junit/ui/editor/ruler/TestMethodMarker.java (3)

34-34: Корректный импорт UIMessages!

Замена JUnitMessages на UIMessages соответствует целям рефакторинга.


62-62: Корректная замена сообщения об ошибке!

Использование UIMessages.TestMethodMarker_MarkersCleanError сохраняет семантику сообщения и соответствует новой структуре.


80-80: Корректная замена сообщения об ошибке создания маркера!

Использование UIMessages.TestMethodMarker_MarkerCreationError логично дополняет общую структуру обработки ошибок.

viewer/src/main/java/ru/biatech/edt/junit/ui/report/CounterPanel.java (2)

27-27: Корректная замена импорта!

Замена JUnitMessages на UIMessages соответствует цели рефакторинга по удалению неиспользуемых объектов.


52-54: Корректное обновление констант сообщений!

Замена констант сообщений на новые из UIMessages выполнена правильно, сохраняя исходную функциональность.

viewer/src/main/java/ru/biatech/edt/junit/ui/dialogs/CompareResultDialog.java (5)

56-56: Корректное добавление импорта!

Импорт UIMessages соответствует общей структуре организации пакетов.


221-221: Корректная замена источника текста заголовка!

Замена JUnitMessages на UIMessages выполнена правильно, функциональность сохранена.


226-226: Корректная замена источника текста кнопки!

Замена JUnitMessages на UIMessages для текста кнопки выполнена правильно, логика создания кнопки не изменилась.


252-254: Корректная замена источников текста меток сравнения!

Замена JUnitMessages на UIMessages для меток expected/actual выполнена правильно, конфигурация сравнения сохранена корректно.


Line range hint 1-277: Общая оценка изменений положительная!

Миграция с JUnitMessages на UIMessages проведена последовательно и корректно. Все изменения соответствуют цели PR по удалению неиспользуемых объектов. Функциональность диалога сохранена, код стал более организованным.

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowWebStackTraceAction.java (3)

20-20: Корректное обновление импорта.

Замена импорта с JUnitMessages на UIMessages соответствует общему рефакторингу в кодовой базе.


25-25: Корректное обновление конструктора.

Обновление параметра конструктора с JUnitMessages на UIMessages согласуется с изменением импорта. Функциональность класса осталась без изменений.


20-25: Проверить согласованность изменений.

Поскольку это часть систематического обновления с JUnitMessages на UIMessages, давайте проверим корректность изменений.

✅ Verification successful

Изменения корректны и согласованы

Проверка подтвердила, что:

  • Ключ сообщения ShowWebStackTraceAction_name присутствует в UIMessages
  • Миграция с JUnitMessages выполнена полностью
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the message key exists in UIMessages and no JUnitMessages references remain

# Check if the message key exists in UIMessages
rg "ShowWebStackTraceAction_name" -A 1 -B 1

# Verify no remaining JUnitMessages references in this file
rg "JUnitMessages" viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowWebStackTraceAction.java

Length of output: 2041

viewer/src/main/java/ru/biatech/edt/junit/launcher/ui/LaunchConfigurationTab.java (4)

33-33: Изменение импорта соответствует рефакторингу.

Замена JUnitMessages на UIMessages согласуется с общим направлением рефакторинга в проекте.


95-95: Корректное обновление источника сообщений.

Изменение источника сообщения для метки вкладки соответствует новой структуре сообщений.


126-126: Обновление логирования ошибок.

Обновление источника сообщения об ошибке соответствует общей стратегии рефакторинга.


Line range hint 33-126: Проверить полноту миграции сообщений.

Хотя изменения корректны, рекомендуется убедиться, что все сообщения из JUnitMessages были перенесены в UIMessages.

✅ Verification successful

Миграция сообщений выполнена полностью

Проверка показала, что все сообщения успешно перенесены в UIMessages. Единственное упоминание JUnitMessages - это корректное указание имени bundle в UIMessages.java, что является частью стандартного механизма работы с ресурсами Java.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify complete migration of messages
# Search for any remaining references to JUnitMessages

rg "JUnitMessages" --type java

Length of output: 805

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowNextFailureAction.java (3)

20-20: Корректная замена импорта!

Замена JUnitMessages на UIMessages соответствует целям рефакторинга и улучшает организацию кода.


31-32: Корректное обновление конструктора!

Использование UIMessages для метки и всплывающей подсказки согласуется с общей стратегией рефакторинга сообщений пользовательского интерфейса.


31-32: Проверить наличие ключей сообщений

Необходимо убедиться, что ключи ShowNextFailureAction_label и ShowNextFailureAction_tooltip определены в классе UIMessages.

✅ Verification successful

Ключи сообщений корректно определены

Проверка показала, что ключи ShowNextFailureAction_label и ShowNextFailureAction_tooltip правильно определены в классе UIMessages и имеют соответствующие переводы в файлах ресурсов.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the message keys exist in UIMessages

# Search for the message keys in UIMessages
rg "ShowNextFailureAction_(label|tooltip)" --type java

Length of output: 1224

viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/FailureViewer.java (3)

32-32: Корректное обновление импорта.

Замена импорта с JUnitMessages на UIMessages соответствует задаче по удалению неиспользуемых объектов и централизации сообщений в UIMessages.


121-121: Корректное обновление источника текста метки.

Изменение источника текста метки с JUnitMessages на UIMessages согласуется с обновлением импорта. Функциональность осталась прежней, изменился только источник сообщения.


32-32: Проверить корректность переноса сообщений.

Необходимо убедиться, что все сообщения корректно перенесены из JUnitMessages в UIMessages.

Also applies to: 121-121

✅ Verification successful

Перенос сообщений выполнен корректно

  • Ключ TestRunnerViewPart_label_failure присутствует в UIMessages с корректными локализациями
  • Ссылка на JUnitMessages в BUNDLE_NAME является корректной, так как указывает на файл ресурсов
  • Остальные упоминания JUnitMessages находятся в закомментированном коде
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Проверка оставшихся ссылок на JUnitMessages
rg "JUnitMessages" 

# Проверка наличия ключа в UIMessages
rg "TestRunnerViewPart_label_failure" -g "UIMessages*"

Length of output: 1287

viewer/src/main/java/ru/biatech/edt/junit/ui/testitemaction/RunTestCaseAction.java (1)

24-24: Корректная замена JUnitMessages на UIMessages

Замена использования JUnitMessages на UIMessages выполнена правильно. Изменения соответствуют цели рефакторинга.

Давайте проверим полное удаление класса JUnitMessages:

Also applies to: 38-38, 48-48

✅ Verification successful

Подтверждение удаления JUnitMessages

Класс JUnitMessages успешно удален из кодовой базы. Остались только:

  • Корректная ссылка на бандл ресурсов в UIMessages.java
  • Неактивный закомментированный код в TestViewer.java, который можно удалить в рамках отдельной задачи по очистке кода
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify complete removal of JUnitMessages class
# Expected: No results, confirming complete removal

# Search for any remaining references to JUnitMessages
rg "JUnitMessages" 

# Search for any remaining JUnitMessages class file
fd "JUnitMessages.java"

Length of output: 817


Script:

#!/bin/bash
# Get context around the commented lines in TestViewer.java
rg -B 5 -A 5 "JUnitMessages" viewer/src/main/java/ru/biatech/edt/junit/ui/report/TestViewer.java

Length of output: 803

viewer/src/main/java/ru/biatech/edt/junit/ui/testitemaction/DebugTestCaseAction.java (2)

38-38: LGTM! Корректная замена источника сообщений

Замена JUnitMessages.Debug_Test_Label на UIMessages.Debug_Test_Label соответствует целям рефакторинга по консолидации сообщений.

Also applies to: 48-48


24-24: Проверьте полноту миграции с JUnitMessages на UIMessages

Убедитесь, что все необходимые сообщения были перенесены из JUnitMessages в UIMessages.

✅ Verification successful

Миграция с JUnitMessages на UIMessages выполнена корректно

  • Класс UIMessages содержит все необходимые константы сообщений
  • Активных ссылок на старый класс JUnitMessages не обнаружено
  • Файл ресурсов правильно подключен через BUNDLE_NAME
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Проверка использования JUnitMessages в других файлах
rg "JUnitMessages" --type java

Length of output: 805


Script:

#!/bin/bash
# Check UIMessages.java content
cat viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages.java

Length of output: 8221

viewer/src/main/java/ru/biatech/edt/junit/ui/editor/ruler/TestMethodActionDelegate.java (4)

41-41: Корректное обновление импорта!

Замена импорта с JUnitMessages на UIMessages соответствует общей структуре рефакторинга.


160-160: Корректное обновление текста меню!

Обновление метки запуска теста на UIMessages.Run_Test_Label соответствует новой структуре сообщений.


167-167: Корректное обновление текста меню отладки!

Обновление метки отладки теста на UIMessages.Debug_Test_Label соответствует новой структуре сообщений.


195-195: Проверьте обработку ошибок!

Обновление сообщения об ошибке выполнено корректно, но рекомендуется проверить, что сообщение определено в новом классе UIMessages.

✅ Verification successful

Проверка обработки ошибок завершена успешно!

Сообщение об ошибке TestMethodActionDelegate_CollectingMarkers корректно определено в классе UIMessages и имеет соответствующие переводы.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the error message constant exists in UIMessages
rg -A 1 "TestMethodActionDelegate_CollectingMarkers" --type java

Length of output: 1409

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowPreviousFailureAction.java (1)

20-20: Корректное добавление импорта UIMessages

Замена JUnitMessages на UIMessages соответствует общей стратегии рефакторинга в проекте.

viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/actions/CopyTraceAction.java (3)

29-29: Корректное обновление импорта!

Замена JUnitMessages на UIMessages выполнена правильно и соответствует целям рефакторинга.


41-41: Корректное обновление метки действия!

Обновление ссылки на сообщение с JUnitMessages.CopyTrace_action_label на UIMessages.CopyTrace_action_label выполнено корректно.


Line range hint 1-82: Проверка полного удаления JUnitMessages.

Давайте убедимся, что в файле не осталось неучтённых ссылок на JUnitMessages.

✅ Verification successful

Проверка завершена: ссылки на JUnitMessages полностью удалены

Все ссылки на сообщения корректно используют класс UIMessages. Оставшихся ссылок на JUnitMessages не обнаружено.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify complete removal of JUnitMessages references
rg "JUnitMessages" "viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/actions/CopyTraceAction.java"

Length of output: 103


Script:

#!/bin/bash
# Check UIMessages import and usage
rg "UIMessages" "viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/actions/CopyTraceAction.java"

# Search for any message-related strings to ensure complete migration
rg "Messages|\.getMessage" "viewer/src/main/java/ru/biatech/edt/junit/ui/stacktrace/actions/CopyTraceAction.java"

Length of output: 694

viewer/src/main/java/ru/biatech/edt/junit/ui/PluginUI.java (1)

37-37: Проверьте влияние переименования класса на кодовую базу.

Переименование класса с JUnitUI на PluginUI выглядит логичным, так как новое название лучше отражает общее назначение класса. Однако необходимо убедиться, что все зависимые классы были обновлены.

Выполните следующий скрипт для проверки использования старого названия класса:

✅ Verification successful

Переименование класса выполнено корректно

Проверка показала, что переименование класса с JUnitUI на PluginUI было выполнено последовательно во всей кодовой базе. Все ссылки на класс используют новое название, и интеграция с другими компонентами плагина работает корректно.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that there are no remaining references to JUnitUI

# Search for any remaining references to JUnitUI
rg "JUnitUI" --type java

Length of output: 24


Script:

#!/bin/bash
# Check git history for the file and current usage
git log --oneline viewer/src/main/java/ru/biatech/edt/junit/ui/PluginUI.java

# Search for current PluginUI references
rg "PluginUI" --type java -A 2

# Search for potential class references with common patterns
rg "class.*UI" --type java

Length of output: 3531

viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages.java (3)

151-153: Корректная реализация конструктора.

Приватный конструктор с поясняющим комментарием соответствует лучшим практикам для утилитарного класса с сообщениями.


147-149: Корректное обновление инициализации и удаление неиспользуемой переменной.

Изменения соответствуют цели PR по удалению неиспользуемого кода. Инициализация сообщений корректно обновлена для работы с новым именем класса.


Line range hint 1-154: Проверка удаления строковой константы.

Давайте убедимся, что удаленная константа TestSessionLabelProvider_testName_JUnitVersion действительно нигде не используется.

✅ Verification successful

Подтверждение безопасного удаления константы

Удаление константы TestSessionLabelProvider_testName_JUnitVersion является безопасным, так как она не используется нигде в кодовой базе.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for any usage of the removed constant

# Search in Java files
rg "TestSessionLabelProvider_testName_JUnitVersion" -t java

# Search in properties files
rg "TestSessionLabelProvider_testName_JUnitVersion" -t properties

Length of output: 160


Script:

#!/bin/bash
# Search without file type restriction to catch all possible occurrences
rg "TestSessionLabelProvider_testName_JUnitVersion"

# Search specifically in files with .properties extension
find . -name "*.properties" -exec grep -l "TestSessionLabelProvider_testName_JUnitVersion" {} \;

# Search for partial matches in case the key is split or formatted differently
rg "TestSessionLabelProvider.*JUnitVersion"

Length of output: 197

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowTimeAction.java (2)

20-20: Подтверждаю изменение импорта.

Замена JUnitMessages на UIMessages соответствует общей стратегии рефакторинга по централизации сообщений в приложении.


25-25: Корректное обновление конструктора.

Использование UIMessages.TestRunnerViewPart_show_execution_time вместо JUnitMessages.TestRunnerViewPart_show_execution_time согласуется с изменением импорта и сохраняет исходную функциональность.

Давайте проверим согласованность изменений в других файлах:

✅ Verification successful

Изменение корректно реализовано

Проверка подтверждает, что замена JUnitMessages на UIMessages выполнена правильно. Константа TestRunnerViewPart_show_execution_time присутствует в новом классе и его файлах локализации, а старые ссылки полностью удалены из кодовой базы.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Описание: Проверка использования UIMessages вместо JUnitMessages

# Поиск оставшихся упоминаний JUnitMessages
rg "JUnitMessages\.TestRunnerViewPart_show_execution_time" 

# Поиск новых использований UIMessages
rg "UIMessages\.TestRunnerViewPart_show_execution_time"

Length of output: 292


Script:

#!/bin/bash
# Поиск определения константы в UIMessages
ast-grep --pattern 'TestRunnerViewPart_show_execution_time = $_'

# Поиск файла UIMessages
fd UIMessages

Length of output: 280


Script:

#!/bin/bash
# Проверка содержимого файлов локализации и класса сообщений
echo "=== UIMessages.java ==="
cat viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages.java

echo -e "\n=== UIMessages.properties ==="
cat viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages.properties

echo -e "\n=== UIMessages_ru.properties ==="
cat viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages_ru.properties

Length of output: 32029

viewer/src/main/java/ru/biatech/edt/junit/ui/UIPreferencesConstants.java (2)

30-30: Переименование класса выполнено корректно

Класс переименован в соответствии с общим рефакторингом, убрана избыточная часть названия "JUnit". Изменение конструктора также выполнено корректно.

Also applies to: 39-41


35-35: Проверить влияние изменения PLUGIN_ID на существующие настройки

Замена JUnitUI.PLUGIN_ID на PluginUI.PLUGIN_ID в константе может привести к потере существующих пользовательских настроек, так как путь хранения в preferences изменится.

Выполните проверку:

✅ Verification successful

Изменение PLUGIN_ID безопасно

Замена JUnitUI.PLUGIN_ID на PluginUI.PLUGIN_ID не повлияет на существующие настройки, так как фактическое значение ID остается неизменным: "ru.biatech.edt.junit". Путь хранения настроек в preferences определяется значением ID, а не именем класса, в котором оно определено.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Поиск использования старого ID в других файлах
rg "JUnitUI.PLUGIN_ID" -l

# Поиск кода миграции настроек
rg "preferences?.migrate|preferences?.transfer" -l

Length of output: 78


Script:

#!/bin/bash
# Поиск определения класса JUnitUI
ast-grep --pattern 'class JUnitUI'

# Поиск файлов с "preference" в имени
fd -t f -i preference

# Поиск определений PLUGIN_ID
rg "PLUGIN_ID.*=.*" -A 1

# Поиск использования preferences в коде
rg -i "\.getpreference|\.setpreference|preferencestore" -A 2

Length of output: 2651

viewer/src/main/java/ru/biatech/edt/junit/launcher/v8/LaunchHelper.java (4)

35-35: Корректная замена импорта!

Замена JUnitMessages на UIMessages соответствует цели рефакторинга и улучшает организацию кода.


163-163: Проверить ключ сообщения об отсутствии конфигурации

Необходимо проверить наличие ключа LaunchHelper_DefaultLaunchConfigurationNotFound в UIMessages.

✅ Verification successful

Ключ сообщения корректно определен

Ключ LaunchHelper_DefaultLaunchConfigurationNotFound присутствует в файлах ресурсов с корректными переводами для английского и русского языков.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Проверка наличия ключа сообщения в файлах ресурсов
rg "LaunchHelper_DefaultLaunchConfigurationNotFound" -t properties

Length of output: 101


Script:

#!/bin/bash
# Поиск файлов .properties и проверка ключа сообщения
fd -e properties -x rg "LaunchHelper_DefaultLaunchConfigurationNotFound" {}

# Поиск определения класса UIMessages
ast-grep --pattern 'class UIMessages {
  $$$
}'

Length of output: 615


174-174: Проверить ключ заголовка диалога

Необходимо проверить наличие ключа LaunchTest_title в UIMessages.

Also applies to: 183-183

✅ Verification successful

Ключ заголовка диалога корректно определен

Ключ LaunchTest_title найден в файлах локализации с корректными значениями для русского и английского языков.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Проверка наличия ключа заголовка в файлах ресурсов
rg "LaunchTest_title" -t properties

Length of output: 70


Script:

#!/bin/bash
# Поиск ключа сообщения во всех файлах
rg "LaunchTest_title" -C 2

Length of output: 4879


116-118: Проверить наличие ключей сообщений

Необходимо убедиться, что ключи LaunchHelper_LaunchConfigurationNotSpecified и LaunchHelper_LaunchConfigurationNotFound существуют в UIMessages.

✅ Verification successful

Проверка пройдена успешно

Ключи сообщений LaunchHelper_LaunchConfigurationNotSpecified и LaunchHelper_LaunchConfigurationNotFound корректно определены в файлах локализации с соответствующими переводами для английского и русского языков.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Проверка наличия ключей сообщений в файлах ресурсов
rg "LaunchHelper_LaunchConfigurationNotSpecified|LaunchHelper_LaunchConfigurationNotFound" -t properties

Length of output: 141


Script:

#!/bin/bash
# Найти все файлы .properties и искать в них ключи сообщений
fd -e properties -x grep -l "LaunchHelper_LaunchConfigurationNotSpecified\|LaunchHelper_LaunchConfigurationNotFound" {}

Length of output: 264


Script:

#!/bin/bash
# Показать содержимое файлов свойств, отфильтровав только нужные ключи
echo "=== Default messages ==="
grep "LaunchHelper_LaunchConfigurationNotSpecified\|LaunchHelper_LaunchConfigurationNotFound" "./viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages.properties"
echo -e "\n=== Russian messages ==="
grep "LaunchHelper_LaunchConfigurationNotSpecified\|LaunchHelper_LaunchConfigurationNotFound" "./viewer/src/main/java/ru/biatech/edt/junit/ui/UIMessages_ru.properties"

Length of output: 1347

viewer/src/main/java/ru/biatech/edt/junit/ui/report/history/RunnerViewHistory.java (6)

27-27: Корректный импорт нового типа Session!

Добавление импорта соответствует переходу на новую модель управления сессиями.


41-41: Корректное обновление типового параметра!

Замена типового параметра на Session соответствует общей стратегии рефакторинга.


81-82: Проверьте корректность получения сессий!

Метод getHistoryEntries() использует старый способ получения сессий через TestViewerPlugin.core(). Рекомендуется использовать инжектированный SessionsManager.

 public List<Session> getHistoryEntries() {
-    return TestViewerPlugin.core().getSessionsManager().getSessions();
+    return testRunnerViewPart.getSessionsManager().getSessions();
 }

91-95: Несоответствие названия метода новому типу!

Метод setActiveTestRunSession в названии все еще содержит упоминание старого типа TestRunSession, хотя работает с новым типом Session.

 public void setActiveEntry(Session entry) {
-    var deactivatedSession = testRunnerViewPart.setActiveTestRunSession(entry);
+    var deactivatedSession = testRunnerViewPart.setActiveSession(entry);
     if (deactivatedSession != null) {
       deactivatedSession.swapOut();
     }
 }

Line range hint 114-136: Корректная обработка состояний сессии!

Метод getImageDescriptor правильно обрабатывает все возможные состояния сессии и корректно использует новый тип.


140-147: Корректная реализация форматирования текста!

Метод getText правильно использует новые методы типа Session для получения имени и времени запуска.

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/FailuresOnlyFilterAction.java (1)

20-20: Корректная замена источника сообщений!

Замена JUnitMessages на UIMessages выполнена правильно. Все ссылки на сообщения обновлены корректно.

Also applies to: 26-27

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ShowTestHierarchyAction.java (1)

20-20: Правильное обновление зависимостей!

Переход с JUnitMessages на UIMessages выполнен корректно. Все ссылки в конструкторе обновлены соответствующим образом.

Also applies to: 27-27

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/ToggleOrientationAction.java (1)

20-20: Последовательное обновление сообщений в switch-конструкции!

Замена источника сообщений с JUnitMessages на UIMessages проведена корректно во всех ветках switch-конструкции. Структура кода осталась неизменной.

Also applies to: 30-30, 34-34, 38-38

viewer/src/main/java/ru/biatech/edt/junit/ui/report/actions/CopyFailureListAction.java (1)

27-27: Корректное обновление сообщений в действиях и обработке ошибок!

Замена JUnitMessages на UIMessages выполнена правильно во всех местах использования:

  • В конструкторе при установке метки действия
  • В диалоге обработки ошибок при проблемах с буфером обмена

Логика обработки ошибок осталась без изменений.

Also applies to: 41-41, 58-58

viewer/src/main/java/ru/biatech/edt/junit/model/SessionsManager.java (2)

77-77: ⚠️ Potential issue

Замените assert на явную проверку null

Использование assert для проверки null небезопасно, так как проверки могут быть отключены в рабочей среде.

Примените следующий дифф для исправления:

-      assert session != null;
+      if (session == null) {
+          TestViewerPlugin.log().logError("Session is null after import");
+          return;
+      }

Likely invalid or redundant comment.


82-82: Проверьте необходимость удаления файла отчета

Удаление файла отчета сразу после его загрузки может привести к потере данных, если файл потребуется для отладки.

Добавьте проверку перед удалением:

-      Files.deleteIfExists(reportPath);
+      if (!Preferences.isKeepReportFiles()) {
+          Files.deleteIfExists(reportPath);
+      }

Comment on lines 99 to 107
public void setHistoryEntries(List<Session> remainingEntries, Session activeEntry) {
testRunnerViewPart.setActiveTestRunSession(activeEntry);

List<TestRunSession> testRunSessions = TestViewerPlugin.core().getModel().getTestRunSessions();
testRunSessions.removeAll(remainingEntries);
for (TestRunSession testRunSession : testRunSessions) {
TestViewerPlugin.core().getModel().removeTestRunSession(testRunSession);
var sessions = TestViewerPlugin.core().getSessionsManager().getSessions();
sessions.removeAll(remainingEntries);
for (var session : sessions) {
TestViewerPlugin.core().getSessionsManager().removeSession(session);
}
for (TestRunSession remaining : remainingEntries) {
for (var remaining : remainingEntries) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Оптимизируйте управление сессиями!

В методе setHistoryEntries происходит неоптимальное управление сессиями:

  1. Возможны проблемы при удалении сессий из списка во время итерации
  2. Излишнее получение списка сессий через глобальный доступ

Предлагаемый рефакторинг:

 public void setHistoryEntries(List<Session> remainingEntries, Session activeEntry) {
     testRunnerViewPart.setActiveTestRunSession(activeEntry);
 
-    var sessions = TestViewerPlugin.core().getSessionsManager().getSessions();
-    sessions.removeAll(remainingEntries);
-    for (var session : sessions) {
-      TestViewerPlugin.core().getSessionsManager().removeSession(session);
-    }
+    var sessionsManager = testRunnerViewPart.getSessionsManager();
+    var sessionsToRemove = sessionsManager.getSessions().stream()
+        .filter(session -> !remainingEntries.contains(session))
+        .toList();
+    
+    sessionsToRemove.forEach(sessionsManager::removeSession);
 
     for (var remaining : remainingEntries) {
       remaining.swapOut();
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void setHistoryEntries(List<Session> remainingEntries, Session activeEntry) {
testRunnerViewPart.setActiveTestRunSession(activeEntry);
List<TestRunSession> testRunSessions = TestViewerPlugin.core().getModel().getTestRunSessions();
testRunSessions.removeAll(remainingEntries);
for (TestRunSession testRunSession : testRunSessions) {
TestViewerPlugin.core().getModel().removeTestRunSession(testRunSession);
var sessions = TestViewerPlugin.core().getSessionsManager().getSessions();
sessions.removeAll(remainingEntries);
for (var session : sessions) {
TestViewerPlugin.core().getSessionsManager().removeSession(session);
}
for (TestRunSession remaining : remainingEntries) {
for (var remaining : remainingEntries) {
public void setHistoryEntries(List<Session> remainingEntries, Session activeEntry) {
testRunnerViewPart.setActiveTestRunSession(activeEntry);
var sessionsManager = testRunnerViewPart.getSessionsManager();
var sessionsToRemove = sessionsManager.getSessions().stream()
.filter(session -> !remainingEntries.contains(session))
.toList();
sessionsToRemove.forEach(sessionsManager::removeSession);
for (var remaining : remainingEntries) {
remaining.swapOut();
}
}

Comment on lines +112 to +118
public void startSession(LifecycleItem item) {
var session = new Session(item.getName(), LaunchHelper.getProject(item.getTestLaunch().getLaunchConfiguration()));
session.setLaunch(item.getTestLaunch());
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Добавьте проверки на null

Отсутствуют проверки на null для параметров, что может привести к NullPointerException.

Примените следующий дифф для добавления проверок:

   public void startSession(LifecycleItem item) {
+      if (item == null || item.getTestLaunch() == null || 
+          item.getTestLaunch().getLaunchConfiguration() == null) {
+          TestViewerPlugin.log().logError("Invalid lifecycle item");
+          return;
+      }
       var session = new Session(item.getName(), LaunchHelper.getProject(item.getTestLaunch().getLaunchConfiguration()));
       session.setLaunch(item.getTestLaunch());
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void startSession(LifecycleItem item) {
var session = new Session(item.getName(), LaunchHelper.getProject(item.getTestLaunch().getLaunchConfiguration()));
session.setLaunch(item.getTestLaunch());
}
public void startSession(LifecycleItem item) {
if (item == null || item.getTestLaunch() == null ||
item.getTestLaunch().getLaunchConfiguration() == null) {
TestViewerPlugin.log().logError("Invalid lifecycle item");
return;
}
var session = new Session(item.getName(), LaunchHelper.getProject(item.getTestLaunch().getLaunchConfiguration()));
session.setLaunch(item.getTestLaunch());
}

Comment on lines +212 to +227
private void notifySessionRemoved(Session session) {
session.stopTestRun();
var launch = session.getLaunch();
if (launch != null) {
ILaunchManager launchManager = DebugPlugin.getDefault().getLaunchManager();
var launchManager = DebugPlugin.getDefault().getLaunchManager();
launchManager.removeLaunch(launch);
}

fTestRunSessionListeners.forEach(it -> it.sessionRemoved(testRunSession));
sessionListeners.forEach(it -> it.sessionRemoved(session));
}

private void notifyTestRunSessionAdded(TestRunSession testRunSession) {
fTestRunSessionListeners.forEach(it -> it.sessionAdded(testRunSession));
private void notifySessionAdded(Session session) {
sessionListeners.forEach(it -> it.sessionAdded(session));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Добавьте обработку ошибок при вызове слушателей

Отсутствует обработка исключений при вызове методов слушателей, что может привести к прерыванию цепочки уведомлений.

Примените следующий дифф:

   private void notifySessionRemoved(Session session) {
     session.stopTestRun();
     var launch = session.getLaunch();
     if (launch != null) {
       var launchManager = DebugPlugin.getDefault().getLaunchManager();
       launchManager.removeLaunch(launch);
     }
 
-    sessionListeners.forEach(it -> it.sessionRemoved(session));
+    sessionListeners.forEach(it -> {
+      try {
+        it.sessionRemoved(session);
+      } catch (Exception e) {
+        TestViewerPlugin.log().logError("Error notifying listener", e);
+      }
+    });
   }
 
   private void notifySessionAdded(Session session) {
-    sessionListeners.forEach(it -> it.sessionAdded(session));
+    sessionListeners.forEach(it -> {
+      try {
+        it.sessionAdded(session);
+      } catch (Exception e) {
+        TestViewerPlugin.log().logError("Error notifying listener", e);
+      }
+    });
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private void notifySessionRemoved(Session session) {
session.stopTestRun();
var launch = session.getLaunch();
if (launch != null) {
ILaunchManager launchManager = DebugPlugin.getDefault().getLaunchManager();
var launchManager = DebugPlugin.getDefault().getLaunchManager();
launchManager.removeLaunch(launch);
}
fTestRunSessionListeners.forEach(it -> it.sessionRemoved(testRunSession));
sessionListeners.forEach(it -> it.sessionRemoved(session));
}
private void notifyTestRunSessionAdded(TestRunSession testRunSession) {
fTestRunSessionListeners.forEach(it -> it.sessionAdded(testRunSession));
private void notifySessionAdded(Session session) {
sessionListeners.forEach(it -> it.sessionAdded(session));
private void notifySessionRemoved(Session session) {
session.stopTestRun();
var launch = session.getLaunch();
if (launch != null) {
var launchManager = DebugPlugin.getDefault().getLaunchManager();
launchManager.removeLaunch(launch);
}
sessionListeners.forEach(it -> {
try {
it.sessionRemoved(session);
} catch (Exception e) {
TestViewerPlugin.log().logError("Error notifying listener", e);
}
});
}
private void notifySessionAdded(Session session) {
sessionListeners.forEach(it -> {
try {
it.sessionAdded(session);
} catch (Exception e) {
TestViewerPlugin.log().logError("Error notifying listener", e);
}
});

Comment on lines +339 to +342
testElement.pushErrorInfo(testStatus, errorInfo.message, errorInfo.type, errorInfo.getTrace(), errorInfo.getExpected(), errorInfo.getActual());
session.registerTestFailureStatus(testElement);
errorInfo.clean(false);
fStatus = null;
testStatus = null;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Добавьте валидацию данных об ошибке

Отсутствует проверка данных об ошибке перед их сохранением, что может привести к некорректным данным в отчете.

Примените следующий дифф:

   private void handleFailure(TestElement testElement) {
+    if (errorInfo == null || testElement == null) {
+      TestViewerPlugin.log().logError("Invalid error info or test element");
+      return;
+    }
     testElement.pushErrorInfo(testStatus, errorInfo.message, errorInfo.type, errorInfo.getTrace(), errorInfo.getExpected(), errorInfo.getActual());
     session.registerTestFailureStatus(testElement);
     errorInfo.clean(false);
     testStatus = null;
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
testElement.pushErrorInfo(testStatus, errorInfo.message, errorInfo.type, errorInfo.getTrace(), errorInfo.getExpected(), errorInfo.getActual());
session.registerTestFailureStatus(testElement);
errorInfo.clean(false);
fStatus = null;
testStatus = null;
if (errorInfo == null || testElement == null) {
TestViewerPlugin.log().logError("Invalid error info or test element");
return;
}
testElement.pushErrorInfo(testStatus, errorInfo.message, errorInfo.type, errorInfo.getTrace(), errorInfo.getExpected(), errorInfo.getActual());
session.registerTestFailureStatus(testElement);
errorInfo.clean(false);
testStatus = null;

Comment on lines +88 to 94
if (locator != null && progressMonitor != null) {
int line = locator.getLineNumber();
if (line - 20 >= lastReportedLine) {
line -= line % 20;
fLastReportedLine = line;
fMonitor.subTask(NLS.bind(JUnitMessages.TestRunHandler_lines_read, line));
lastReportedLine = line;
progressMonitor.subTask(NLS.bind(UIMessages.TestRunHandler_lines_read, line));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Добавьте проверку границ для расчета строк

Текущая реализация может вызвать арифметическое исключение при делении на ноль или переполнении.

Примените следующий дифф:

     if (locator != null && progressMonitor != null) {
       int line = locator.getLineNumber();
-      if (line - 20 >= lastReportedLine) {
+      if (line > 0 && line - 20 >= lastReportedLine) {
         line -= line % 20;
         lastReportedLine = line;
         progressMonitor.subTask(NLS.bind(UIMessages.TestRunHandler_lines_read, line));
       }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (locator != null && progressMonitor != null) {
int line = locator.getLineNumber();
if (line - 20 >= lastReportedLine) {
line -= line % 20;
fLastReportedLine = line;
fMonitor.subTask(NLS.bind(JUnitMessages.TestRunHandler_lines_read, line));
lastReportedLine = line;
progressMonitor.subTask(NLS.bind(UIMessages.TestRunHandler_lines_read, line));
}
if (locator != null && progressMonitor != null) {
int line = locator.getLineNumber();
if (line > 0 && line - 20 >= lastReportedLine) {
line -= line % 20;
lastReportedLine = line;
progressMonitor.subTask(NLS.bind(UIMessages.TestRunHandler_lines_read, line));
}

Comment on lines +799 to 801
return status.isErrorOrFailure() || (!session.isRunning() && status == TestStatus.RUNNING); // rerunning
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Добавьте проверку на null для session

Отсутствует проверка на null для session, что может привести к NullPointerException.

Примените следующий дифф:

     public boolean select(TestElement testElement) {
       TestStatus status = testElement.getStatus();
-      return status.isErrorOrFailure() || (!session.isRunning() && status == TestStatus.RUNNING);  // rerunning
+      return status.isErrorOrFailure() || (session != null && !session.isRunning() && status == TestStatus.RUNNING);
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return status.isErrorOrFailure() || (!session.isRunning() && status == TestStatus.RUNNING); // rerunning
}
}
return status.isErrorOrFailure() || (session != null && !session.isRunning() && status == TestStatus.RUNNING); // rerunning
}
}

Comment on lines +1145 to 1155
private class IsRunningJob extends Job {
public IsRunningJob(String name) {
super(name);
setSystem(true);
}

@Override
public IStatus run(IProgressMonitor monitor) {
// wait until the test run terminates
fJUnitIsRunningLock.acquire();
isRunningLock.acquire();
return Status.OK_STATUS;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Добавьте корректную обработку блокировки

Отсутствует освобождение блокировки в случае прерывания задачи, что может привести к взаимной блокировке.

Примените следующий дифф:

     @Override
     public IStatus run(IProgressMonitor monitor) {
-      // wait until the test run terminates
-      isRunningLock.acquire();
+      try {
+        // wait until the test run terminates
+        isRunningLock.acquire();
+      } catch (Exception e) {
+        TestViewerPlugin.log().logError("Error acquiring lock", e);
+      }
       return Status.OK_STATUS;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private class IsRunningJob extends Job {
public IsRunningJob(String name) {
super(name);
setSystem(true);
}
@Override
public IStatus run(IProgressMonitor monitor) {
// wait until the test run terminates
fJUnitIsRunningLock.acquire();
isRunningLock.acquire();
return Status.OK_STATUS;
private class IsRunningJob extends Job {
public IsRunningJob(String name) {
super(name);
setSystem(true);
}
@Override
public IStatus run(IProgressMonitor monitor) {
try {
// wait until the test run terminates
isRunningLock.acquire();
} catch (Exception e) {
TestViewerPlugin.log().logError("Error acquiring lock", e);
}
return Status.OK_STATUS;
}

Comment on lines +1262 to 1264
if (session != null) {
fTestViewer.setSortingCriterion(this.sortingCriterion);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Обеспечьте потокобезопасность при обновлении UI

Обновление UI может происходить из фонового потока, что может привести к исключению SWTException.

Примените следующий дифф:

     public void setSortingCriterion(SortingCriterion sortingCriterion) {
       this.sortingCriterion = sortingCriterion;
       if (session != null) {
-        fTestViewer.setSortingCriterion(this.sortingCriterion);
+        Display.getDefault().asyncExec(() -> {
+          if (!fTestViewer.getTestViewerControl().isDisposed()) {
+            fTestViewer.setSortingCriterion(this.sortingCriterion);
+          }
+        });
       }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (session != null) {
fTestViewer.setSortingCriterion(this.sortingCriterion);
}
public void setSortingCriterion(SortingCriterion sortingCriterion) {
this.sortingCriterion = sortingCriterion;
if (session != null) {
Display.getDefault().asyncExec(() -> {
if (!fTestViewer.getTestViewerControl().isDisposed()) {
fTestViewer.setSortingCriterion(this.sortingCriterion);
}
});
}
}

@alkoleft alkoleft merged commit cd349d8 into develop Jan 15, 2025
1 check was pending
@alkoleft alkoleft deleted the feature/57 branch January 15, 2025 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Убрать страницу параметров
1 participant