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

fix: sync self pkg #532

Merged
merged 1 commit into from
Jun 25, 2023
Merged

fix: sync self pkg #532

merged 1 commit into from
Jun 25, 2023

Conversation

elrrrrrrr
Copy link
Member

During the syncUpstream process, it will attempt to create sync repeatedly until it times out, when the pkg has been published in the self registry.

  1. 🐞 When executing the syncTask, filter out scenarios where the target registry is the self registry.

包迁移至当前 registry 时,收到同步请求会产生无效的同步任务,当 syncUpstream 时,会尝试重复创建 sync 直到超时。

  1. 🐞 syncTask 执行时,先过滤目标 registry 是当前 registry 的场景。

@elrrrrrrr elrrrrrrr requested a review from fengmk2 June 25, 2023 14:43
task.taskId, task.targetName);
return;
}

if (pkg && pkg?.registryId !== registry?.registryId) {
if (pkg.registryId) {
logs.push(`[${isoNow()}] ❌❌❌❌❌ ${fullname} registry is ${pkg.registryId} not belong to ${registry?.registryId}, skip sync ❌❌❌❌❌`);

Choose a reason for hiding this comment

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

这段代码的变更是将 SyncDeleteMode 移动到 constants.ts 中并添加了 PresetRegistryName 常量。

对于代码中的修改,似乎没有风险或错误。添加PreserRegistryName 可以使代码更加清晰且易于维护。

在行号为 372 处,如果 registry?.name 是 PresetRegistryName.self 则不需要执行同步操作,会记录一条日志并退出该方法。这样做使代码结构更清晰,也可以避免意外的同步错误。

@elrrrrrrr elrrrrrrr requested a review from killagu June 25, 2023 14:43
assert(log.includes(`${name} has been published to the self registry, skip sync ❌❌❌❌❌`));

});

it('should updated package manifests when version insert duplicated', async () => {
// https://www.npmjs.com/package/@cnpmcore/test-sync-package-has-two-versions
const name = '@cnpmcore/test-sync-package-has-two-versions';

Choose a reason for hiding this comment

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

这段代码是针对 test/core/service/PackageSyncerService/executeTask.test.ts 中的一个测试用例。在添加一个新的测试用例,用于确保当同步任务是自注册表时,跳过同步。在测试之前,该用例创建了一个名为 @cnpmcore/test-self-sync 的包,并将它发布到自注册表中进行测试。测试执行后,通过读取任务日志来验证是否成功地跳过了同步任务,并输出消息“已将***发布到自注册表中,跳过同步”。建议对日志编写更加详细的测试用例,以方便调试和验证。

@elrrrrrrr elrrrrrrr requested a review from gemwuu June 25, 2023 14:44
Copy link
Contributor

@killagu killagu left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Jun 25, 2023

Codecov Report

Merging #532 (b65ee6d) into master (c9d9ce8) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #532      +/-   ##
==========================================
- Coverage   97.03%   96.97%   -0.06%     
==========================================
  Files         174      174              
  Lines       16546    16554       +8     
  Branches     2168     2165       -3     
==========================================
- Hits        16055    16054       -1     
- Misses        491      500       +9     
Impacted Files Coverage Δ
app/core/service/PackageSyncerService.ts 98.22% <100.00%> (+0.01%) ⬆️

... and 2 files with indirect coverage changes

@fengmk2 fengmk2 merged commit ada3e22 into master Jun 25, 2023
12 of 13 checks passed
@fengmk2 fengmk2 deleted the sync-self branch June 25, 2023 15:02
fengmk2 pushed a commit that referenced this pull request Jun 25, 2023
[skip ci]

## [3.34.7](v3.34.6...v3.34.7) (2023-06-25)

### Bug Fixes

* sync self pkg ([#532](#532)) ([ada3e22](ada3e22))
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.

3 participants