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.

Sign up for GitHub

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

Jump to bottom

enable ts strict in node #2450

Merged
merged 30 commits into from
Jul 2, 2024
Merged

enable ts strict in node #2450

merged 30 commits into from
Jul 2, 2024

Conversation

yoozo
Copy link
Collaborator

@yoozo yoozo commented Jun 14, 2024 •
edited
Loading

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)
Enable ts strict in node

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist

  • I have tested locally
  • I have performed a self review of my changes
  • Updated any relevant documentation
  • Linked to any relevant issues
  • I have added tests relevant to my changes
  • Any dependent changes have been merged and published in downstream modules
  • My code is up to date with the base branch
  • I have updated relevant changelogs. We suggest using chan

@yoozo yoozo requested review from stwiname and jiqiang90 June 14, 2024 08:17
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 14, 2024 •
edited
Loading

Coverage report for .

Caution

Test run failed

St.❔
 Category Percentage Covered / Total
🟡 Statements
66.56% (-3.55% 🔻)
 19312/29014
🟡 Branches
79.23% (+0.94% 🔼)
 2468/3115
🟡 Functions
67.97% (+5.47% 🔼)
 1076/1583
🟡 Lines
66.56% (-3.55% 🔻)
 19312/29014
Show new covered files 🐣
St.❔
 File Statements Branches Functions Lines
🟢
... / migrate.ts
 85.09% 18.18% 100% 85.09%
🟢
... / migrate-abis.controller.ts
 95% 87.5% 100% 95%
🟢
... / constants.ts
 100% 100% 100% 100%
🟢
... / index.ts
 100% 100% 100% 100%
🟢
... / ethereum.ts
 82.05% 83.33% 100% 82.05%
🟢
... / migrate-manifest.controller.ts
 88% 73.33% 88.89% 88%
🟢
... / migrate-mapping.controller.ts
 100% 100% 100% 100%
🟢
... / migrate-controller.ts
 92.31% 84.62% 100% 92.31%
🟢
... / migrate.fixtures.ts
 100% 100% 100% 100%
🟢
... / migrate-schema.controller.ts
 100% 100% 100% 100%
🔴
... / types.ts
 0% 0% 0% 0%
🟢
... / createProject.fixtures.ts
 94.64% 75% 100% 94.64%
🟢
... / networkFamily.ts
 92.59% 88.89% 100% 92.59%
🟢
... / admin.controller.ts
 98.62% 96.55% 92.86% 98.62%
🟢
... / blockRange.ts
 100% 100% 100% 100%
🟢
... / index.ts
 100% 100% 100% 100%
🟢
... / dictionary.fixtures.ts
 100% 100% 100% 100%
🟡
... / monitor.service.ts
 69.61% 86.9% 94.12% 69.61%
🔴
... / sandbox.service.ts
 33.33% 50% 25% 33.33%
🔴
... / worker.monitor.service.ts
 41.46% 100% 0% 41.46%
🔴
... / process.ts
 50% 66.67% 40% 50%
🔴
... / foreceClean.init.ts
 47.62% 100% 0% 47.62%
🔴
... / reindex.init.ts
 34.48% 100% 0% 34.48%
🟢
... / string.ts
 100% 100% 100% 100%
🔴
... / admin.module.ts
 0% 0% 0% 0%
🔴
... / datasourceProcessors.ts
 0% 0% 0% 0%
🔴
... / index.ts
 0% 0% 0% 0%
🔴
... / types.ts
 0% 0% 0% 0%
Show files with reduced coverage 🔻
St.❔
 File Statements Branches Functions Lines
🔴
... / deploy.ts
19.21% (-1.93% 🔻)
 100%
50% (+50% 🔼)
19.21% (-1.93% 🔻)
🔴
... / publish.ts
44% (-45.33% 🔻)
40% (-17.14% 🔻)
 100%
44% (-45.33% 🔻)
🔴
... / deploy-controller.ts
32.49% (-4.98% 🔻)
100% (+22.22% 🔼)
12.5% (-12.5% 🔻)
32.49% (-4.98% 🔻)
🟢
... / generate-controller.ts
 97.65%
93.85% (-0.18% 🔻)
 100% 97.65%
🟡
... / init-controller.ts
72.89% (+2.4% 🔼)
54.55% (-13.45% 🔻)
61.11% (+7.78% 🔼)
72.89% (+2.4% 🔼)
🔴
... / project-controller.ts
17.81% (-79.45% 🔻)
100% (+40% 🔼)
0% (-100% 🔻)
17.81% (-79.45% 🔻)
🟡
... / publish-controller.ts
60.98% (-19.27% 🔻)
71.88% (+0.76% 🔼)
50% (-50% 🔻)
60.98% (-19.27% 🔻)
🟢
... / models.ts
96.11% (-3.33% 🔻)
96% (+3.69% 🔼)
92.31% (-7.69% 🔻)
96.11% (-3.33% 🔻)
🔴
... / utils.ts
55.93% (-3.68% 🔻)
 100%
28.57% (-4.76% 🔻)
55.93% (-3.68% 🔻)
🟢
... / ProjectManifestVersioned.ts
82.19% (-2.74% 🔻)
88.89% (-11.11% 🔻)
61.54% (+3.21% 🔼)
82.19% (-2.74% 🔻)
🟢
... / model.ts
95.43% (-2.29% 🔻)
 100%
80% (+8.57% 🔼)
95.43% (-2.29% 🔻)
🔴
... / models.ts
47.06% (-52.94% 🔻)
 100%
0% (-100% 🔻)
47.06% (-52.94% 🔻)
🔴
... / load.ts
 59.18%
50% (-5.56% 🔻)
 66.67% 59.18%
🟡
... / github-reader.ts
65.52% (-23.77% 🔻)
85.71% (+10.71% 🔼)
71.43% (-11.9% 🔻)
65.52% (-23.77% 🔻)
🟢
... / reader.ts
81.58% (-7.89% 🔻)
57.14% (-17.86% 🔻)
 100%
81.58% (-7.89% 🔻)
🟡
... / utils.ts
67.91% (+5.21% 🔼)
69.77% (-3.4% 🔻)
60% (+4% 🔼)
67.91% (+5.21% 🔼)
🟢
... / base.ts
95.33% (-2.67% 🔻)
92.31% (-7.69% 🔻)
90% (+10% 🔼)
95.33% (-2.67% 🔻)
🔴
... / models.ts
49.37% (-50.63% 🔻)
 100%
37.5% (-62.5% 🔻)
49.37% (-50.63% 🔻)
🟢
... / NodeConfig.ts
88.48% (-0.12% 🔻)
77.59% (-2.04% 🔻)
86.96% (+0.91% 🔼)
88.48% (-0.12% 🔻)
🟡
... / ProjectUpgrade.service.ts
73.91% (-13.16% 🔻)
93.59% (+2.23% 🔼)
84% (-7.3% 🔻)
73.91% (-13.16% 🔻)
🟢
... / db.module.ts
89.06% (+0.78% 🔼)
62.5% (-20.83% 🔻)
 80%
89.06% (+0.78% 🔼)
🟡
... / migration-helpers.ts
75.28% (-0.74% 🔻)
88.24% (+7.64% 🔼)
 90.91%
75.28% (-0.74% 🔻)
🟡
... / sequelizeUtil.ts
79.46% (-11.61% 🔻)
65.38% (-4.99% 🔻)
 62.5%
79.46% (-11.61% 🔻)
🟢
... / sync-helper.ts
85.42% (-2.96% 🔻)
 88.04%
73.77% (-1.23% 🔻)
85.42% (-2.96% 🔻)
🟡
... / StoreOperations.ts
72.94% (-19.37% 🔻)
86.36% (+1.36% 🔼)
87.5% (-12.5% 🔻)
72.94% (-19.37% 🔻)
🔴
... / benchmark.service.ts
43.36% (-11.89% 🔻)
 66.67%
36.36% (-3.64% 🔻)
43.36% (-11.89% 🔻)
🔴
... / base-block-dispatcher.ts
19% (-28.35% 🔻)
 100%
4.35% (-5.65% 🔻)
19% (-28.35% 🔻)
🔴
... / block-dispatcher.ts
26.98% (-14.01% 🔻)
 66.67%
25% (-3.57% 🔻)
26.98% (-14.01% 🔻)
🔴
... / worker-block-dispatcher.ts
36.17% (-14.04% 🔻)
 100%
25% (-2.27% 🔻)
36.17% (-14.04% 🔻)
🔴
... / coreDictionary.ts
45.33% (-54.67% 🔻)
87.5% (-6.94% 🔻)
77.78% (-22.22% 🔻)
45.33% (-54.67% 🔻)
🟡
... / dictionary.service.ts
67% (-18.15% 🔻)
80% (-5.71% 🔻)
83.33% (-7.58% 🔻)
67% (-18.15% 🔻)
🟡
... / dictionaryV1.ts
71.43% (-21.8% 🔻)
73.81% (-4.45% 🔻)
91.67% (-8.33% 🔻)
71.43% (-21.8% 🔻)
🟡
... / dictionaryV2.ts
67.37% (-14.31% 🔻)
68% (+2.78% 🔼)
88.89% (-11.11% 🔻)
67.37% (-14.31% 🔻)
🔴
... / ds-processor.service.ts
39.37% (-22.72% 🔻)
 75%
58.33% (-29.17% 🔻)
39.37% (-22.72% 🔻)
🔴
... / dynamic-ds.service.ts
59.84% (-30.35% 🔻)
65% (-1.67% 🔻)
80% (-5.71% 🔻)
59.84% (-30.35% 🔻)
🟢
... / inMemoryCache.service.ts
86.21% (-13.79% 🔻)
 100%
75% (-25% 🔻)
86.21% (-13.79% 🔻)
🔴
... / indexer.manager.ts
16.13% (-17.08% 🔻)
 100%
10% (-2.5% 🔻)
16.13% (-17.08% 🔻)
🟢
... / PoiBlock.ts
83.87% (-14.52% 🔻)
 96.55%
75% (-25% 🔻)
83.87% (-14.52% 🔻)
🔴
... / poi.service.ts
56.64% (+32.71% 🔼)
55.56% (-44.44% 🔻)
80% (+51.43% 🔼)
56.64% (+32.71% 🔼)
🔴
... / poiModel.ts
40% (-23.64% 🔻)
 71.43%
44.44% (-12.7% 🔻)
40% (-23.64% 🔻)
🔴
... / poiSync.service.ts
55.88% (-31% 🔻)
77.61% (+1.05% 🔼)
88.89% (-3.42% 🔻)
55.88% (-31% 🔻)
🟡
... / project.service.ts
66.45% (-13.55% 🔻)
71.93% (+1.75% 🔼)
80.77% (-3.23% 🔻)
66.45% (-13.55% 🔻)
🟡
... / sandbox.ts
61.24% (-6.74% 🔻)
 57.14%
57.14% (-4.4% 🔻)
61.24% (-6.74% 🔻)
🔴
... / smartBatch.service.ts
10.43% (-13.91% 🔻)
 100%
12.5% (-1.79% 🔻)
10.43% (-13.91% 🔻)
🔴
... / entity.ts
19.23% (-42.31% 🔻)
 100% 0%
19.23% (-42.31% 🔻)
🔴
... / store.ts
13.11% (-19.09% 🔻)
 100% 0%
13.11% (-19.09% 🔻)
🟡
... / cacheModel.ts
68.89% (-16.18% 🔻)
85.92% (+7.6% 🔼)
71.43% (-4.76% 🔻)
68.89% (-16.18% 🔻)
🟡
... / cachePoi.ts
75.53% (-18.09% 🔻)
 88.89%
71.43% (-11.9% 🔻)
75.53% (-18.09% 🔻)
🟢
... / cacheable.ts
89.66% (-0.97% 🔻)
 75% 100%
89.66% (-0.97% 🔻)
🟡
... / csvStore.service.ts
76.92% (-14.1% 🔻)
 80%
80% (-20% 🔻)
76.92% (-14.1% 🔻)
🟢
... / test.runner.ts
94.9% (-4.43% 🔻)
77.78% (+4.44% 🔼)
75% (-25% 🔻)
94.9% (-4.43% 🔻)
🔴
... / testing.service.ts
12% (-16.57% 🔻)
 100% 0%
12% (-16.57% 🔻)
🔴
... / worker.builder.ts
59.83% (-19.36% 🔻)
 65.38% 80%
59.83% (-19.36% 🔻)
🔴
... / worker.cache.service.ts
48.78% (-9.76% 🔻)
 100% 0%
48.78% (-9.76% 🔻)
🔴
... / worker.connectionPoolState.manager.ts
38.21% (-22.76% 🔻)
 100% 0%
38.21% (-22.76% 🔻)
🔴
... / worker.dynamic-ds.service.ts
44.44% (-17.78% 🔻)
 100% 0%
44.44% (-17.78% 🔻)
🔴
... / worker.service.ts
20.35% (-24.29% 🔻)
 100% 0%
20.35% (-24.29% 🔻)
🟢
... / worker.store.service.ts
80% (-2.61% 🔻)
 100% 50%
80% (-2.61% 🔻)
🔴
... / worker.ts
41.82% (-0.63% 🔻)
 100% 0%
41.82% (-0.63% 🔻)
🔴
... / worker.unfinalizedBlocks.service.ts
33.33% (-66.67% 🔻)
 100%
0% (-100% 🔻)
33.33% (-66.67% 🔻)
🔴
... / logger.ts
44.74% (-19.74% 🔻)
 33.33%
40% (+20% 🔼)
44.74% (-19.74% 🔻)
🟡
... / meta.service.ts
68.57% (-0.95% 🔻)
100% (+20% 🔼)
36.36% (-3.64% 🔻)
68.57% (-0.95% 🔻)
🔴
... / forceClean.service.ts
22.08% (-4.24% 🔻)
 100% 0%
22.08% (-4.24% 🔻)
🔴
... / reindex.service.ts
14.73% (-18.6% 🔻)
 100% 0%
14.73% (-18.6% 🔻)
🟡
... / autoQueue.ts
60.3% (-28.09% 🔻)
 85.11%
73.08% (-6.09% 🔻)
60.3% (-28.09% 🔻)
🟡
... / blocks.ts
77.78% (-22.22% 🔻)
 88.89%
66.67% (-33.33% 🔻)
77.78% (-22.22% 🔻)
🟢
... / promise.ts
86.44% (-5.32% 🔻)
100% (+13.64% 🔼)
71.43% (-14.29% 🔻)
86.44% (-5.32% 🔻)
🔴
... / yargs.ts
44.38% (-1.11% 🔻)
 100% 14.29%
44.38% (-1.11% 🔻)
🟡
... / apiPromise.connection.ts
76.19% (+7.11% 🔼)
50% (-30% 🔻)
62.5% (+19.64% 🔼)
76.19% (+7.11% 🔼)
🟡
... / block-dispatcher.service.ts
66.35% (-2.52% 🔻)
 100%
33.33% (+8.33% 🔼)
66.35% (-2.52% 🔻)
🔴
... / worker-block-dispatcher.service.ts
23.65% (-13.31% 🔻)
 100% 0%
23.65% (-13.31% 🔻)
🟢
... / substrateDictionary.service.ts
93.4% (+2.83% 🔼)
38.46% (-34.27% 🔻)
100% (+20% 🔼)
93.4% (+2.83% 🔼)
🟢
... / substrateDictionaryV1.ts
85.19% (-0.3% 🔻)
78.26% (+8.49% 🔼)
84.62% (+1.28% 🔼)
85.19% (-0.3% 🔻)
🟡
... / substrateDictionaryV2.ts
70.79% (-4.72% 🔻)
50% (-25% 🔻)
 33.33%
70.79% (-4.72% 🔻)
🟢
... / fetch.module.ts
90% (-1.88% 🔻)
50% (-25% 🔻)
 100%
90% (-1.88% 🔻)
🔴
... / indexer.manager.ts
56.35% (-1.2% 🔻)
 100%
20% (+10.91% 🔼)
56.35% (-1.2% 🔻)
🟡
... / base-runtime.service.ts
68.5% (-3.82% 🔻)
76.92% (+1.92% 🔼)
63.64% (-11.36% 🔻)
68.5% (-3.82% 🔻)
🟢
... / runtimeService.ts
94.52% (+2.31% 🔼)
81.82% (-7.07% 🔻)
100% (+20% 🔼)
94.52% (+2.31% 🔼)
🔴
... / http.ts
49.03% (-30.89% 🔻)
 40%
28.57% (-2.2% 🔻)
49.03% (-30.89% 🔻)
🟢
... / project.ts
88.73% (-0.16% 🔻)
 68.18% 87.5%
88.73% (-0.16% 🔻)
🟢
... / buffer.ts
95.12% (-4.88% 🔻)
95.65% (-4.35% 🔻)
85.71% (+1.5% 🔼)
95.12% (-4.88% 🔻)
🟢
... / builder.ts
 92.77%
82.14% (-0.62% 🔻)
 100% 92.77%
🟢
... / entities.ts
85.4% (-2.92% 🔻)
74.77% (-11.31% 🔻)
 100%
85.4% (-2.92% 🔻)
🟡
... / logger.ts
69.01% (-0.59% 🔻)
50% (-5% 🔻)
78.57% (+3.57% 🔼)
69.01% (-0.59% 🔻)

Test suite run failed

Failed tests: 6/558. Failed suites: 3/98. 
  ● Codegen can generate schema › Should dedupe enums

    ENOENT: no such file or directory, open '/home/runner/work/subql/subql/packages/cli/test/schemaTest/src/types/models/foo.ts'




  ● Cli publish › should upload appropriate project to IPFS

    Publish project to default IPFS failed

      172 |     const results = ipfsWrite.addAll(contents, {pin: true, cidVersion: 0, wrapWithDirectory: isMultichain});
      173 |     for await (const result of results) {
    > 174 |       fileCidMap.set(result.path, result.cid.toString());
          |               ^
      175 |
      176 |       await ipfsWrite.pin.remote.add(result.cid, {service: PIN_SERVICE}).catch((e) => {
      177 |         console.warn(

      at packages/cli/src/controller/publish-controller.ts:174:15
          at async Promise.all (index 0)
      at async uploadFile (packages/cli/src/controller/publish-controller.ts:177:30)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:100:26)
      at async /home/runner/work/subql/subql/packages/cli/src/controller/publish-controller.ts:105:26
          at async Promise.all (index 3)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:103:9)
          at async Promise.all (index 0)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:91:17)
      at async /home/runner/work/subql/subql/packages/cli/src/controller/publish-controller.ts:105:26
          at async Promise.all (index 5)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:103:9)
      at async uploadToIpfs (packages/cli/src/controller/publish-controller.ts:70:28)
      at async Object.<anonymous> (packages/cli/src/controller/publish-controller.spec.ts:38:21)

    Cause:
    HTTPError: <html>
    <head><title>403 Forbidden</title></head>
    <body>
    <center><h1>403 Forbidden</h1></center>
    <hr><center>nginx</center>
    </body>
    </html>

      175 |
      176 |       await ipfsWrite.pin.remote.add(result.cid, {service: PIN_SERVICE}).catch((e) => {
    > 177 |         console.warn(
          |                      ^
      178 |           `Failed to pin file ${result.path}. There might be problems with this file being accessible later. ${e}`
      179 |         );
      180 |       });

      at Object.errorHandler [as handleError] (node_modules/ipfs-http-client/cjs/src/lib/core.js:84:15)
      at async Client.fetch (node_modules/ipfs-http-client/node_modules/ipfs-utils/src/http.js:161:9)
      at async addAll (node_modules/ipfs-http-client/cjs/src/add-all.js:21:17)
      at async Object.last [as default] (node_modules/it-last/index.js:13:20)
      at async Object.add (node_modules/ipfs-http-client/cjs/src/add.js:18:14)
          at async Promise.all (index 0)
      at async uploadFile (packages/cli/src/controller/publish-controller.ts:177:30)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:100:26)
      at async /home/runner/work/subql/subql/packages/cli/src/controller/publish-controller.ts:105:26
          at async Promise.all (index 3)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:103:9)
          at async Promise.all (index 0)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:91:17)
      at async /home/runner/work/subql/subql/packages/cli/src/controller/publish-controller.ts:105:26
          at async Promise.all (index 5)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:103:9)
      at async uploadToIpfs (packages/cli/src/controller/publish-controller.ts:70:28)
      at async Object.<anonymous> (packages/cli/src/controller/publish-controller.spec.ts:38:21)

  ● Cli publish › Get directory CID from multi-chain project

    Publish project to default IPFS failed

      172 |     const results = ipfsWrite.addAll(contents, {pin: true, cidVersion: 0, wrapWithDirectory: isMultichain});
      173 |     for await (const result of results) {
    > 174 |       fileCidMap.set(result.path, result.cid.toString());
          |               ^
      175 |
      176 |       await ipfsWrite.pin.remote.add(result.cid, {service: PIN_SERVICE}).catch((e) => {
      177 |         console.warn(

      at packages/cli/src/controller/publish-controller.ts:174:15
          at async Promise.all (index 0)
      at async uploadFile (packages/cli/src/controller/publish-controller.ts:177:30)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:100:26)
      at async /home/runner/work/subql/subql/packages/cli/src/controller/publish-controller.ts:105:26
          at async Promise.all (index 1)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:103:9)
      at async uploadToIpfs (packages/cli/src/controller/publish-controller.ts:70:28)
      at async Object.<anonymous> (packages/cli/src/controller/publish-controller.spec.ts:66:24)

    Cause:
    HTTPError: <html>
    <head><title>403 Forbidden</title></head>
    <body>
    <center><h1>403 Forbidden</h1></center>
    <hr><center>nginx</center>
    </body>
    </html>

      175 |
      176 |       await ipfsWrite.pin.remote.add(result.cid, {service: PIN_SERVICE}).catch((e) => {
    > 177 |         console.warn(
          |                      ^
      178 |           `Failed to pin file ${result.path}. There might be problems with this file being accessible later. ${e}`
      179 |         );
      180 |       });

      at Object.errorHandler [as handleError] (node_modules/ipfs-http-client/cjs/src/lib/core.js:84:15)
      at async Client.fetch (node_modules/ipfs-http-client/node_modules/ipfs-utils/src/http.js:161:9)
      at async addAll (node_modules/ipfs-http-client/cjs/src/add-all.js:21:17)
      at async Object.last [as default] (node_modules/it-last/index.js:13:20)
      at async Object.add (node_modules/ipfs-http-client/cjs/src/add.js:18:14)
          at async Promise.all (index 0)
      at async uploadFile (packages/cli/src/controller/publish-controller.ts:177:30)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:100:26)
      at async /home/runner/work/subql/subql/packages/cli/src/controller/publish-controller.ts:105:26
          at async Promise.all (index 1)
      at async replaceFileReferences (packages/cli/src/controller/publish-controller.ts:103:9)
      at async uploadToIpfs (packages/cli/src/controller/publish-controller.ts:70:28)
      at async Object.<anonymous> (packages/cli/src/controller/publish-controller.spec.ts:66:24)


  ● Intergration test - Publish › overwrites any exisiting CID files

    thrown: "Exceeded timeout of 300000 ms for a test.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      28 |   it('overwrites any exisiting CID files', async () => {
      29 |     const initCID = 'QmWLxg7xV7ZWUyc7ZxZ8XuQQ7NmH8WQGXzg7VZ3QQNqF-testing';
    > 30 |     const cidFilePath = path.resolve(projectDir, '.project-cid');
         |     ^
      31 |     await fs.promises.writeFile(cidFilePath, initCID);
      32 |     await Publish.run(['-f', projectDir, '-o']);
      33 |     const cidValue = await fs.promises.readFile(cidFilePath, 'utf8');

      at packages/cli/src/commands/publish.test.ts:30:5
      at Object.<anonymous> (packages/cli/src/commands/publish.test.ts:15:1)

  ● Intergration test - Publish › create ipfsCID file stored in local with dictiory path

    thrown: "Exceeded timeout of 300000 ms for a test.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      36 |
      37 |   it('create ipfsCID file stored in local with dictiory path', async () => {
    > 38 |     await Publish.run(['-f', projectDir]);
         |     ^
      39 |     const cidFile = path.resolve(projectDir, '.project-cid');
      40 |     const fileExists = fs.existsSync(cidFile);
      41 |     const IPFScontent = await fs.promises.readFile(cidFile, 'utf8');

      at packages/cli/src/commands/publish.test.ts:38:5
      at Object.<anonymous> (packages/cli/src/commands/publish.test.ts:15:1)

  ● Intergration test - Publish › file name consistent with manfiest file name, if -f <manifest path> is used

    thrown: "Exceeded timeout of 300000 ms for a test.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      45 |
      46 |   // Run this last because it modifies the project
    > 47 |   it('file name consistent with manfiest file name, if -f <manifest path> is used', async () => {
         |     ^
      48 |     const manifestPath = path.resolve(projectDir, 'project.yaml');
      49 |     const testManifestPath = path.resolve(projectDir, 'test.yaml');
      50 |     fs.renameSync(manifestPath, testManifestPath);

      at packages/cli/src/commands/publish.test.ts:47:5
      at Object.<anonymous> (packages/cli/src/commands/publish.test.ts:15:1)

Report generated by 🧪jest coverage report action from 7fe76c7

stwiname
stwiname reviewed Jun 16, 2024
View reviewed changes
packages/node/src/configure/SubqueryProject.ts Outdated
Comment on lines 149 to 157
let schemaString: string | undefined;
try {
schemaString = await reader.getFile(projectManifest.schema.file);
} catch (e) {
throw new Error(
`unable to fetch the schema from ${projectManifest.schema.file}`,
);
}
assert(schemaString, 'Schema file is empty');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs better formatting but i think this is a cleaner way to write this.

Suggested change
let schemaString: string | undefined;
try {
schemaString = await reader.getFile(projectManifest.schema.file);
} catch (e) {
throw new Error(
`unable to fetch the schema from ${projectManifest.schema.file}`,
);
}
assert(schemaString, 'Schema file is empty');
const schemaString = await reader.getFile(projectManifest.schema.file)
.catch(e => {
throw new Error(
`unable to fetch the schema from ${projectManifest.schema.file}`,
);
});

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here, we need to merge the common PR first and then make adjustments.

packages/node/src/configure/SubqueryProject.ts Outdated
@@ -171,7 +172,7 @@ async function loadProjectFromManifestBase(
projectManifest.templates,
root,
reader,
isCustomDs,
isCustomDs, // Note: ts-strict change --- Not sure it should be modified
Copy link
Collaborator

Choose a reason for hiding this comment

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

Im not sure what the comment is here for, there are no changes?

packages/node/src/indexer/api.service.ts Outdated
Comment on lines 109 to 112
private fetchBlocksFunction!: FetchFunc;
private fetchBlocksBatches: GetFetchFunc = () => this.fetchBlocksFunction;
private currentBlockHash: string;
private currentBlockNumber: number;
private currentBlockHash!: string;
private currentBlockNumber!: number;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use getter/setter pattern here

packages/node/src/indexer/api.service.ts Outdated
Comment on lines 159 to 163
connection: IApiConnectionSpecific<
ApiPromise,
ApiAt,
IBlock<BlockContent>[] | IBlock<LightBlockContent>[]
>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the 4th generic for BaseApiService needs to be set to ApiPromiseConnection rather than this change

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fix

packages/node/src/indexer/blockDispatcher/block-dispatcher.service.ts Outdated
Comment on lines 86 to 89
assert(isFullBlock(block.block), 'Block must be a full block');
const runtimeVersion = await this.runtimeService.getRuntimeVersion(
block.block.block,
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a change in behaviour, if its a light block then this will no longer work.

packages/node/src/indexer/dictionary/v1/substrateDictionaryV1.ts Outdated
@@ -221,7 +223,7 @@ export class SubstrateDictionaryV1 extends DictionaryV1<SubstrateDataSource> {
return buildDictionaryV1QueryEntries(dataSources, this.getDsProcessor);
}

parseSpecVersions(raw: SpecVersionDictionary): SpecVersion[] {
parseSpecVersions(raw: SpecVersionDictionary | undefined): SpecVersion[] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should prefer optional instead of | undefined where possible. Applies to other places

Suggested change
parseSpecVersions(raw: SpecVersionDictionary | undefined): SpecVersion[] {
parseSpecVersions(raw?: SpecVersionDictionary): SpecVersion[] {

packages/node/src/indexer/runtime/runtime.service.spec.ts Outdated
@@ -24,7 +25,7 @@ const specVersions: SpecVersion[] = [
{ id: '12', start: 443964, end: 528470 },
{ id: '13', start: 528471, end: 687751 },
{ id: '14', start: 687752, end: 746085 },
{ id: '15', start: 746086, end: null },
{ id: '15', start: 746086, end: 99999999 },
Copy link
Collaborator

Choose a reason for hiding this comment

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

End should be able to be null, i think the types need to be fixed rather than the test changed

packages/node/src/indexer/worker/worker.service.ts Outdated
Comment on lines 71 to 74
assert(isFullBlock(block.block), 'Block should be Full block');
const runtimeVersion = await this.workerRuntimeService.getRuntimeVersion(
block.block.block,
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is also a change in behaviour

packages/node/src/utils/project.ts Outdated
} else {
// If it is stored in ipfs or other resources, we will use the corresponding reader to read the file
// Because ipfs not provide extension of the file, it is difficult to determine its format
// We will use yaml.load to try to load the script and parse them to supported chain types
// if it failed, we will give it another another attempt, and assume the script written in js
// we will download it to a temp folder, and load them within sandbox
const res = await reader.getFile(file);
assert(res, `File ${file} not found`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think getFile should be updated to throw rather than return undefined

packages/node/src/utils/substrate.ts Outdated
Comment on lines 383 to 390
let parentSpecVersion: number | undefined;
if (overallSpecVer !== undefined) {
parentSpecVersion = overallSpecVer;
} else {
parentSpecVersion = runtimeVersions
? runtimeVersions[idx].specVersion.toNumber()
: undefined;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this could be reverted and optional chaining added.

Suggested change
let parentSpecVersion: number | undefined;
if (overallSpecVer !== undefined) {
parentSpecVersion = overallSpecVer;
} else {
parentSpecVersion = runtimeVersions
? runtimeVersions[idx].specVersion.toNumber()
: undefined;
}
const parentSpecVersion =
overallSpecVer !== undefined
? overallSpecVer
: runtimeVersions?.[idx].specVersion.toNumber();

stwiname
stwiname reviewed Jun 18, 2024
View reviewed changes
packages/node-core/src/utils/project.ts Outdated
@@ -17,7 +17,7 @@ import {exitWithError} from '../process';

const logger = getLogger('Project-Utils');

export async function getValidPort(argvPort: number): Promise<number> {
export async function getValidPort(argvPort: number | undefined): Promise<number> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
export async function getValidPort(argvPort: number | undefined): Promise<number> {
export async function getValidPort(argvPort?: number): Promise<number> {

packages/node/src/configure/SchemaMigration.service.test.ts Outdated
@@ -58,11 +58,11 @@ describe('SchemaMigration integration tests', () => {
await apiService.init();
await projectService.init(1);

const dbResults = await sequelize.query(
const dbResults: string[][] = await sequelize.query(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use generics wtih sequelize.query?

packages/node/src/configure/SubqueryProject.ts Outdated
@@ -72,7 +72,7 @@ export class SubqueryProject implements ISubqueryProject {
this.#dataSources = await insertBlockFiltersCronSchedules(
this.dataSources,
getTimestamp,
isRuntimeDs,
isRuntimeDs, // Note: ts-strict change --- Not sure it should be modified
Copy link
Collaborator

Choose a reason for hiding this comment

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

Theres no change here, can we remove this comment?

stwiname
stwiname reviewed Jun 19, 2024
View reviewed changes
packages/node/src/configure/SubqueryProject.ts Outdated
@@ -171,7 +167,7 @@ async function loadProjectFromManifestBase(
projectManifest.templates,
root,
reader,
isCustomDs,
isCustomDs as any, // TODO: There is no good way to temporarily
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to fix this type issue

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

type Big = { a: string, b: number }
type Smail = Big & { c: boolean }

type TypeGuard<T> = (x: any) => x is T;

let F1: TypeGuard<Big> = (x: Big | Smail): x is Big => {
    return true
}

let F2: TypeGuard<Smail> = (x: Big | Smail): x is Smail => {
    return true
}

F2 = F1;    // Type 'Big' is not assignable to type 'Smail'.
F1 = F2;

This change is like assigning a large set to a smaller set, which should be impossible to achieve.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unless we define all our common-<network> isCustomDs as smaller sets. In that case, we would need to modify all common-<network> isCustomDs.

packages/node/src/indexer/api.service.ts Outdated
Comment on lines 134 to 136
private set fetchBlocksFunction(value: FetchFunc) {
this._fetchBlocksFunction = value;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

If these setters are only done once, I think the init function can just set the private var directly

packages/node/src/indexer/dictionary/substrateDictionary.service.ts Outdated
@@ -59,7 +59,7 @@ export class SubstrateDictionaryService extends DictionaryService<
this.nodeConfig,
this.dsProcessorService.getDsProcessor.bind(
this.dsProcessorService,
),
) as any, // TODO: There is no good way to temporarily
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to resolve this

packages/node/src/indexer/dictionary/v1/substrateDictionaryV1.ts Outdated
Comment on lines 134 to 135
} else {
filterList = [handler.filter];
if (handler.filter) filterList = [handler.filter];
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could become an else if

packages/node/src/indexer/worker/worker.ts
@@ -33,7 +33,7 @@ import { WorkerService } from './worker.service';

const logger = getLogger(`worker #${threadId}`);

async function initWorker(startHeight: number): Promise<void> {
async function initWorker(startHeight?: number): Promise<void> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How come this is now optional? I think its required

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image Should this be modified since it is defined as optional here?But the startHeight in the context where this method is used is always optional.

packages/node/src/utils/project.ts Outdated
@@ -43,14 +44,17 @@ export async function getChainTypes(
// If the project is load from local, we will direct load them
let raw: unknown;
if (reader instanceof LocalReader) {
raw = loadChainTypes(file, root);
return loadChainTypes(file, root) as ChainTypes;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please rebase, there was a fix to this code recently

packages/node/tsconfig.json
@@ -4,7 +4,8 @@
"sourceMap": true,
"tsBuildInfoFile": "dist/.tsbuildinfo",
"rootDir": "src",
"outDir": "./dist"
"outDir": "./dist",
"strict": true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because this is the last package in the repo to enable strict, we can set it on the base tsconfig and remove the explicit option on each of the packages

stwiname
stwiname reviewed Jun 23, 2024
View reviewed changes
packages/node/src/indexer/apiPromise.connection.ts Outdated
@@ -58,7 +58,7 @@ export class ApiPromiseConnection
fetchBlocksBatches: GetFetchFunc,
args: { chainTypes: RegisteredTypes },
): Promise<ApiPromiseConnection> {
let provider: ProviderInterface;
let provider: ProviderInterface | undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this could be improved.

Provider is required, but we currently don't throw if there is an unsupported protocol. I think this can be updated to throw

packages/node/src/indexer/dictionary/substrateDictionary.service.ts Outdated
@@ -101,3 +104,26 @@ export class SubstrateDictionaryService extends DictionaryService<
return dict.getSpecVersions();
}
}

function convertGetDsProcessor(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a functional change and should have a test.

I think we will end up implementing this over for each SDK which isn't ideal.

Could this not be fixed by improvements to the dictionary types? E.g. doing something like typeof DsProcessorService["getDsProcessor"]

packages/node/src/indexer/dictionary/v1/substrateDictionaryV1.ts Show resolved Hide resolved
packages/node/src/indexer/runtime/runtime.service.spec.ts Outdated

return Promise.resolve({
specVersion: {
toNumber: () => parseInt(id),
toNumber: () => parseInt(specVersion.id),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not related to the PR but this can lead to issues if not provided. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt#radix

Suggested change
toNumber: () => parseInt(specVersion.id),
toNumber: () => parseInt(specVersion.id, 10),

packages/types/src/interfaces.ts Outdated
@@ -8,7 +8,7 @@ import {IEvent} from '@polkadot/types/types';

export interface SubstrateBlock extends SignedBlock {
// parent block's spec version, can be used to decide the correct metadata that should be used for this block.
specVersion: number;
specVersion?: number;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is required. @jiqiang90 can you confirm

stwiname
stwiname reviewed Jun 27, 2024
View reviewed changes
packages/node/src/indexer/store.service.test.ts Outdated
@@ -77,8 +77,8 @@ describe('Store service integration test', () => {
replacements: { schema: schemaName },
},
);
const expectedTables = result.map(
(t: { table_name: string }) => t.table_name,
const expectedTables = (result as { table_name: string }[]).map(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think its better to use generics on sequelize.query

packages/node/src/indexer/store.service.test.ts Outdated
@@ -278,7 +278,7 @@ WHERE

tempDir = (projectService as any).project.root;

const result = await sequelize.query(
const result: { enum_type: string }[] = await sequelize.query(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here for generics

packages/node/src/utils/project.ts Outdated
@@ -51,6 +52,7 @@ export async function getChainTypes(
// if it failed, we will give it another attempt, and assume the script written in js
// we will download it to a temp folder, and load them within sandbox
const res = await reader.getFile(file);
let raw: unknown;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Raw is already defined on line 45

stwiname
stwiname reviewed Jun 30, 2024
View reviewed changes
packages/node/src/indexer/store.service.test.ts
@@ -64,7 +64,7 @@ describe('Store service integration test', () => {

tempDir = (projectService as any).project.root;

const [result] = await sequelize.query(
const result = await sequelize.query<{ table_name: string }>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this right? I thought it would be

Suggested change
const result = await sequelize.query<{ table_name: string }>(
const [result] = await sequelize.query<[{ table_name: string }]>(

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

type: QueryTypes.SELECT
That’s correct because a type parameter was added to the second argument.

packages/node/src/utils/project.ts Outdated
@@ -1,6 +1,7 @@
// Copyright 2020-2024 SubQuery Pte Ltd authors & contributors
// SPDX-License-Identifier: GPL-3.0

import assert from 'assert';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unused

@stwiname stwiname merged commit 122e7ee into main Jul 2, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stwiname stwiname stwiname approved these changes

@jiqiang90 jiqiang90 Awaiting requested review from jiqiang90

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yoozo @stwiname