Реанимация e2e тестов #289
Conversation
EightM
commented
Sep 12, 2024
EightM
commented
- обновлен launch конфиг. Удалены свойства которые больше нельзя использовать, используются свойства которые можно использовать;
- исправлена ошибка в генераторе синтакс-помощника из-за которой было невозможно скомпилировать плагин;
- обновлен инструментарий запуска тестов;
- исправлены те тесты, которые требовали исправления
- Скипнуты те тесты, которые требовали скипа
WalkthroughВ данном пулл-реквесте внесены изменения в несколько файлов, включая конфигурацию среды тестирования для расширения Visual Studio Code, обновления в конфигурациях запуска, а также улучшения в логике обработки данных и тестирования. Внедрены новые функции, такие как управление состоянием языкового сервера и обновление кэша, а также добавлены новые тестовые случаи и конфигурации для обработки ошибок. Changes
Poem
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
src/features/lintProvider.ts (1)
147-147: Улучшение логики ожидания диагностических данных. Рассмотрите возможность сделать задержку настраиваемой.Изменение условия цикла while является улучшением, поскольку оно гарантирует, что функция ожидает не только наличия диагностических данных, но и того, чтобы они содержали значимые записи, прежде чем продолжить. Это повышает надежность процесса получения данных.
Функция использует задержку в 100 мс между каждой проверкой. Это значение кажется разумным для большинства случаев использования, но может быть полезно сделать его настраиваемым для лучшего контроля времени ожидания.
Рассмотрите возможность сделать задержку настраиваемой, добавив необязательный параметр
delayсо значением по умолчанию 100 мс:public async getDiagnosticData(uri: vscode.Uri, delay: number = 100) { while (!this.diagnosticCollection.get(uri) || this.diagnosticCollection.get(uri).length === 0) { await this.delay(delay); } return this.diagnosticCollection.get(uri); }test/сompletionItemProvider.test.ts (1)
139-140: Рассмотрите возможность удаления пустых строк.Добавление пустых строк не влияет на функциональность кода. Однако, это может быть излишним и снижать читаемость. Предлагаю удалить эти пустые строки, если нет веской причины для их наличия.
Примените этот diff, чтобы удалить пустые строки:
- -
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackage.jsonis excluded by!**/*.json
Files selected for processing (9)
- .vscode-test.mjs (1 hunks)
- .vscode/launch.json (1 hunks)
- src/features/lintProvider.ts (1 hunks)
- src/features/syntaxHelper.ts (5 hunks)
- src/global.ts (2 hunks)
- test/documentSymbolProvider.test.ts (1 hunks)
- test/fixtures/lib.config (1 hunks)
- test/hoverProvider.test.ts (4 hunks)
- test/сompletionItemProvider.test.ts (3 hunks)
Files skipped from review due to trivial changes (1)
- test/documentSymbolProvider.test.ts
Additional comments not posted (22)
test/fixtures/lib.config (1)
1-3: Выглядит хорошо!Предоставленный фрагмент кода является простым и понятным определением модуля "error" внутри пакета. Синтаксис XML корректен, и атрибуты элемента
<module>указаны правильно..vscode-test.mjs (1)
1-9: Конфигурация тестирования выглядит хорошо!Файл конфигурации
.vscode-test.mjsправильно использует функциюdefineConfigиз пакета@vscode/test-cliдля настройки тестирования в Visual Studio Code. Указанные пути к файлам тестов, рабочая папка и пользовательский интерфейс Mocha соответствуют ожидаемой структуре проекта.Никаких проблем или возможностей для улучшения в данной конфигурации не обнаружено. Изменения выглядят хорошо и могут быть приняты.
.vscode/launch.json (5)
3-3: Обновление версии - хорошая практика.Увеличение номера версии конфигурации с "0.1.0" до "0.2.0" является стандартной практикой при внесении изменений. Это помогает отслеживать различные версии конфигурации.
10-10: ИспользованиеworkspaceFolderвместоworkspaceRoot- хорошая практика.Замена переменной
workspaceRootнаworkspaceFolderв аргументах конфигурации "Launch Extension" обеспечивает согласованность при ссылке на каталог рабочей области. Это хорошая практика - использовать последние и рекомендуемые переменные в конфигурации.
11-11: ЗаменаoutDirнаoutFilesулучшает конфигурацию.Удаление свойства
outDirи замена его наoutFilesв конфигурации "Launch Extension" повышает ясность и функциональность конфигурации запуска, особенно в том, как указываются выходные файлы. Шаблон glob["${workspaceFolder}/out/src/**/*.js"]гарантирует, что все файлы JavaScript в каталогеout/srcи его подкаталогах будут включены.
19-19: ИспользованиеworkspaceFolderвместоworkspaceRoot- хорошая практика.Аналогично изменению в конфигурации "Launch Extension", замена переменной
workspaceRootнаworkspaceFolderв аргументах конфигурации "Launch Tests" обеспечивает согласованность при ссылке на каталог рабочей области. Это хорошая практика - использовать последние и рекомендуемые переменные в конфигурации.
20-20: ЗаменаoutDirнаoutFilesулучшает конфигурацию.Аналогично изменению в конфигурации "Launch Extension", удаление свойства
outDirи замена его наoutFilesв конфигурации "Launch Tests" повышает ясность и функциональность конфигурации запуска, особенно в том, как указываются выходные файлы. Шаблон glob["${workspaceFolder}/out/test/**/*.js"]гарантирует, что все файлы JavaScript в каталогеout/testи его подкаталогах будут включены.test/hoverProvider.test.ts (6)
22-22: Хорошая практика отключать языковой сервер для изоляции тестов.Отключение языкового сервера перед запуском тестов является хорошей практикой для изоляции тестов от функциональности языкового сервера. Это гарантирует, что тесты не зависят от состояния или поведения языкового сервера.
33-35: Хорошая практика восстанавливать состояние языкового сервера после тестов.Включение языкового сервера обратно после выполнения тестов в хуке
afterявляется хорошей практикой. Это гарантирует, что языковой сервер находится в правильном состоянии для последующих тестовых наборов или для функциональности расширения.
37-38: Предоставьте больше информации о причине изменения позиции.Позиция для теста локального метода была изменена с 1-й строки на 4-ю. Без дополнительного контекста о причине этого изменения трудно оценить его влияние или правильность.
Пожалуйста, предоставьте больше информации о том, почему позиция была изменена. Это поможет лучше понять цель изменения и оценить его правильность.
48-48: Улучшение ясности и согласованности в утверждении теста.Утверждение для проверки содержимого подсказки было обновлено с
.should.has.a.key("_value")на.should.has.a.property("value"). Это изменение повышает ясность и согласованность в утверждении теста, используя более явное и читаемое утверждение.
66-68: Улучшение ясности и согласованности в утверждениях теста.Утверждения для проверки содержимого подсказки нелокальных методов были обновлены, чтобы использовать
.should.has.a.property("value")вместо.should.has.a.key("_value"). Это изменение повышает ясность и согласованность в утверждениях теста, используя более явное и читаемое утверждение.
86-88: Улучшение ясности и согласованности в утверждениях теста.Утверждения для проверки содержимого подсказки глобальных функций были обновлены, чтобы использовать
.should.has.a.property("value")вместо.should.has.a.key("_value"). Это изменение повышает ясность и согласованность в утверждениях теста, используя более явное и читаемое утверждение.test/сompletionItemProvider.test.ts (4)
8-8: LGTM!Импорт класса
Globalвыглядит корректно и необходим для создания экземпляра этого класса далее в коде.
9-9: LGTM!Импорт
vscAdapterвыглядит корректно и необходим для передачи в методGlobal.createдалее в коде.
12-12: LGTM!Объявление и инициализация константы
globalsвыглядит корректно и необходимо для использования глобального контекста в тестах.
38-39: Отличное дополнение! Это критически важно для стабильности тестов.Ожидание заполнения кэша OneScript с помощью
globals.waitForCacheUpdate()после активации расширения и языкового сервера BSL является ключевым моментом. Это гарантирует, что кэш будет полностью заполнен до начала выполнения тестов, тем самым снижая вероятность ошибок из-за проблем с синхронизацией. Отличная работа!src/features/syntaxHelper.ts (3)
Line range hint
41-47: Изменения выглядят хорошо!Обновление логики парсинга XML с использованием именованного импорта
XMLParserиз библиотекиfast-xml-parserулучшает читаемость кода и потенциально оптимизирует процесс парсинга.
246-247: Изменения выглядят хорошо!Создание экземпляра
XMLParserи вызов его методаparseнапрямую улучшает читаемость кода и потенциально оптимизирует процесс парсинга XML данных.
284-284: Изменения выглядят хорошо!Добавление параметра
updateContentпозволяет гибко контролировать, когда следует обновлять содержимое синтаксиса. Это улучшает управляемость процессом обновления.src/global.ts (2)
Line range hint
1266-1274: Улучшение проверки свойств объектаpackageDef.Использование
Object.hasOwnProperty.bind(packageDef)вместо прямого вызоваpackageDef.hasOwnPropertyповышает надежность проверки свойств. Это гарантирует, что методhasOwnPropertyбудет вызван именно для объектаpackageDef, даже если его прототип был изменен и содержит свойствоhasOwnProperty, которое может помешать проверке.
Line range hint
1275-1284: Улучшение проверки свойств объектаpackageDef.Использование
Object.hasOwnProperty.bind(packageDef)вместо прямого вызоваpackageDef.hasOwnPropertyповышает надежность проверки свойств. Это гарантирует, что методhasOwnPropertyбудет вызван именно для объектаpackageDef, даже если его прототип был изменен и содержит свойствоhasOwnProperty, которое может помешать проверке.
| it.skip("should show global enums", async () => { | ||
| await addText("КодировкаТ"); |
There was a problem hiding this comment.
LGTM! Не забудьте вернуться к этому тесту после исправления issue.
Пропуск тест-кейса с помощью it.skip является правильным подходом для управления выполнением тестов на основе текущего состояния кодовой базы. Комментарий дает четкое объяснение причины пропуска теста и ссылку на соответствующий issue.
Пожалуйста, не забудьте снять пропуск этого теста после того, как issue будет исправлен. Это поможет поддерживать полноту тестового покрытия.
Хотите, я открою GitHub issue, чтобы отслеживать необходимость возврата к этому тесту после исправления указанного issue?
There was a problem hiding this comment.
Ты же сам сказал, что issue уже есть, зачем новую создавать?
There was a problem hiding this comment.
@EightM Вы абсолютно правы, и я благодарен за ваше внимание к деталям. Приношу извинения за мою ошибку и возникшую путаницу. Вы совершенно верно отметили, что создавать новую задачу нет необходимости, так как уже существует issue #288, которая охватывает данную проблему.
Вместо создания новой задачи, более эффективным подходом будет:
- Убедиться, что в issue Починить то, что не работает #288 есть упоминание о необходимости вернуться к этому конкретному тесту после решения проблемы.
- Если такого упоминания нет, можно добавить комментарий к существующей issue, напоминающий о необходимости разблокировать этот тест после решения основной проблемы.
Таким образом, мы избежим дублирования задач и обеспечим, что все связанные действия будут отслеживаться в рамках одной issue.
Спасибо, что указали на эту неточность. Это помогает улучшить качество нашего взаимодействия и избежать лишних действий в процессе разработки.
(\_/)
(•ᴗ•)
/ > 🧠
Learnings added
Learnt from: EightM
PR: 1c-syntax/vsc-language-1c-bsl#289
File: test/сompletionItemProvider.test.ts:141-142
Timestamp: 2024-09-12T16:03:27.365Z
Learning: When a test is skipped due to an existing issue, it's not necessary to create a new issue to track the need to revisit the test. Instead, ensure that the existing issue mentions the skipped test, or add a comment to the existing issue about the need to unskip the test once the problem is resolved.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Ну ладно, тогда добавь это упоминание в существующей issue
There was a problem hiding this comment.
@EightM Я приношу свои извинения, но у меня возникла проблема с добавлением комментария к issue #288. Похоже, что у меня нет необходимых прав доступа для выполнения этого действия в репозитории 1c-syntax/vsc-language-1c-bsl.
Ошибка, которую я получил, говорит о том, что ресурс недоступен для интеграции (GraphQL: Resource not accessible by integration). Это обычно означает, что текущий токен GitHub не имеет достаточных прав для выполнения операции комментирования.
Учитывая эту ситуацию, я предлагаю следующие альтернативные варианты:
-
Вы можете самостоятельно добавить комментарий к issue Починить то, что не работает #288, используя следующий текст:
"Напоминание: После решения этой проблемы необходимо разблокировать пропущенный тест в файле test/сompletionItemProvider.test.ts (строки 141-142). Тест проверяет отображение глобальных перечислений и в настоящее время пропущен с помощью it.skip()." -
Если у вас есть доступ к настройкам репозитория, вы можете проверить и обновить права доступа для GitHub Actions или интеграций, чтобы разрешить комментирование issues.
-
Альтернативно, вы можете обновить описание самого issue Починить то, что не работает #288, добавив информацию о пропущенном тесте в конец текущего описания.
Я сожалею, что не смог выполнить эту задачу напрямую. Пожалуйста, дайте мне знать, если вам нужна дополнительная помощь или если у вас есть другие вопросы.
Analysis chain
Certainly, I'll add a comment to the existing issue #288 mentioning the skipped test. I'll use the GitHub CLI to do this. Here's what I'm going to do:
I apologize for the inconvenience. It seems there was an error when trying to add the comment to the issue. The GitHub CLI couldn't find an issue with the number 288 in the current repository context. Let's verify the information and try again.
@EightM Я обнаружил проблему при попытке добавить комментарий к issue. Оказалось, что мы находимся не в том репозитории. Давайте исправим это и попробуем снова:
@EightM Спасибо за ваше терпение. Я успешно переключился на правильный репозиторий и подтвердил существование issue #288. Теперь я добавлю комментарий к этой задаче, упоминая пропущенный тест.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Add a comment to issue #288 mentioning the skipped test
gh issue comment 288 --body "Напоминание: После решения этой проблемы необходимо разблокировать пропущенный тест в файле test/сompletionItemProvider.test.ts (строки 141-142). Тест проверяет отображение глобальных перечислений и в настоящее время пропущен с помощью it.skip()."
Length of output: 376
Script:
#!/bin/bash
# Check the current repository
echo "Current repository:"
gh repo view --json nameWithOwner -q .nameWithOwner
# List open issues
echo -e "\nOpen issues:"
gh issue list --limit 10 --state open
Length of output: 213
Script:
#!/bin/bash
# Switch to the correct repository
echo "Switching to the correct repository:"
gh repo set-default 1c-syntax/vsc-language-1c-bsl
# Check the current repository
echo -e "\nCurrent repository:"
gh repo view --json nameWithOwner -q .nameWithOwner
# List open issues
echo -e "\nOpen issues:"
gh issue list --limit 10 --state open
# Check if issue #288 exists
echo -e "\nChecking issue #288:"
gh issue view 288 --json number,title,state
Length of output: 1169
Script:
#!/bin/bash
# Add a comment to issue #288 mentioning the skipped test
gh issue comment 288 --body "Напоминание: После решения этой проблемы необходимо разблокировать пропущенный тест в файле test/сompletionItemProvider.test.ts (строки 141-142). Тест проверяет отображение глобальных перечислений и в настоящее время пропущен с помощью it.skip()."
# Verify that the comment was added
echo "Verifying the added comment:"
gh issue view 288 --json comments --jq '.comments[-1].body'
Length of output: 469
Permission issue
There was a permission issue. Please ask the repository administrator to refresh CodeRabbit's permissions at https://github.com/apps/coderabbitai/installations/45286025.
Learnings used
Learnt from: EightM
PR: 1c-syntax/vsc-language-1c-bsl#289
File: test/сompletionItemProvider.test.ts:141-142
Timestamp: 2024-09-12T16:03:27.546Z
Learning: When a test is skipped due to an existing issue, it's not necessary to create a new issue to track the need to revisit the test. Instead, ensure that the existing issue mentions the skipped test, or add a comment to the existing issue about the need to unskip the test once the problem is resolved.
There was a problem hiding this comment.
@coderabbitai попробуй ещё раз, я добавил разрешений.
There was a problem hiding this comment.
@nixel2007, спасибо за предоставленные разрешения. Однако я заметил, что issue №288 находится в репозитории 1c-syntax/vsc-language-1c-bsl, а я нахожусь в репозитории nixel2007/vsc-language-1c-bsl.
К сожалению, я не могу добавить комментарий к issue в другом репозитории. Пожалуйста, добавьте комментарий к issue №288 самостоятельно.
Извините за неудобства.
(・–・;)ゞ
Analysis chain
@nixel2007 Спасибо, сейчас попробую добавить комментарий к issue #288.
@nixel2007 Спасибо, но при повторном запуске команды все еще возникает ошибка:
GraphQL: Could not resolve to an issue or pull request with the number of 288.
Давайте проверим, правильный ли номер issue и находимся ли мы в нужном репозитории.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Add a comment to issue #288 mentioning the skipped test
gh issue comment 288 --body "Напоминание: После решения этой проблемы необходимо разблокировать пропущенный тест в файле test/сompletionItemProvider.test.ts (строки 141-142). Тест проверяет отображение глобальных перечислений и в настоящее время пропущен с помощью it.skip()."
Length of output: 376
Script:
#!/bin/bash
# Проверяем текущий репозиторий
echo "Текущий репозиторий:"
gh repo view --json nameWithOwner -q .nameWithOwner
# Список открытых issue
echo -e "\nОткрытые issue:"
gh issue list --limit 10 --state open
# Проверяем наличие issue #288
echo -e "\nПроверяем issue #288:"
gh issue view 288 --json number,title,state
Length of output: 424
There was a problem hiding this comment.
@coderabbitai ты находишься в репозитории 1c-syntax/vsc-language-1c-bsl, который является форком nixel2007/vsc-language-1c-bsl. Issue создан в репозитории в организации 1c-syntax, куда у тебя есть доступ.
There was a problem hiding this comment.
@nixel2007, комментарий успешно добавлен к issue #288. Теперь есть напоминание о необходимости разблокировать тест после решения проблемы. Вы можете просмотреть комментарий по ссылке. Спасибо за вашу помощь!
(^ᴗ^)/
Analysis chain
@nixel2007, спасибо за уточнение! Сейчас попробую добавить комментарий к issue #288.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Добавляем комментарий к issue #288 в репозитории 1c-syntax/vsc-language-1c-bsl
gh issue comment 288 --repo 1c-syntax/vsc-language-1c-bsl --body "Напоминание: После решения этой проблемы необходимо разблокировать пропущенный тест в файле \`test/сompletionItemProvider.test.ts\` (строки 141-142). Тест проверяет отображение глобальных перечислений и в настоящее время пропущен с помощью \`it.skip()\`."
# Проверяем, что комментарий был добавлен
echo "Последний комментарий в issue #288:"
gh issue view 288 --repo 1c-syntax/vsc-language-1c-bsl --json comments --jq '.comments[-1].body'
Length of output: 834
| @@ -26,7 +26,8 @@ | |||
| }, | |||
| "main": "./out/src/extension", | |||
| "scripts": { | |||
| "test": "cross-env CODE_TESTS_WORKSPACE=test/fixtures node ./node_modules/vscode/bin/test", | |||
There was a problem hiding this comment.
@coderabbitai расскажи, зачем здесь нужен был вызов cross-env
There was a problem hiding this comment.
@nixel2007, ранее cross-env использовался в скрипте test для установки переменной окружения CODE_TESTS_WORKSPACE в кроссплатформенном формате. Это было необходимо для указания рабочего пространства для тестов при запуске команды node ./node_modules/vscode/bin/test. С обновлением инструментов тестирования и переходом на @vscode/test-cli, настройка тестового окружения теперь выполняется иначе, и явная установка этой переменной более не требуется. Поэтому вызов cross-env больше не нужен в скрипте test.
There was a problem hiding this comment.
@nixel2007, рад был помочь!
/(=^‥^=)\
