Skip to content

feat(binding/nodejs): upgrade test behavior and infra#3297

Merged
Xuanwo merged 37 commits intoapache:mainfrom
eryue0220:feat/nodejes-test-suit
Nov 1, 2023
Merged

feat(binding/nodejs): upgrade test behavior and infra#3297
Xuanwo merged 37 commits intoapache:mainfrom
eryue0220:feat/nodejes-test-suit

Conversation

@eryue0220
Copy link
Copy Markdown
Contributor

Related: #3066

@Xuanwo Xuanwo changed the title test(binding/nodejs): upgrade test behavior and infra feat(binding/nodejs): upgrade test behavior and infra Oct 16, 2023
Comment thread bindings/nodejs/index.js Outdated
Comment thread bindings/nodejs/tests/test_service.test.js Outdated
Comment thread bindings/nodejs/tests/test_service.test.js Outdated
Comment thread bindings/nodejs/tests/test_service.test.js Outdated
@github-actions github-actions Bot added the releases-note/feat The PR implements a new feature or has a title that begins with "feat" label Oct 16, 2023
…or-opendal into feat/nodejes-test-suit

* 'feat/nodejes-test-suit' of github.com:eryue0220/incubator-opendal: (23 commits)
  docs: Add docs in website for sqlite/mysql/postgresql services (#3290)
  chore(bindings/java): align mapping POJO pattern (#3289)
  refactor(bindings/java): explict error handling (#3288)
  feat(layer/prometheus): Support custom metric bucket for Histogram (#3275)
  feat(bindings/java): convey backtrace on exception (#3286)
  refactor(oay): import dav-server-opendalfs (#3285)
  feat(binding/java): make Metadata a POJO (#3277)
  feat(services/rocksdb): change blocking_x in async_x call to tokio::task::blocking_spawn (#3279)
  feat(services/sled): change blocking_x in async_x call to tokio::task::blocking_spawn (#3280)
  feat(binding/c): Add blocking_reader for C binding (#3259)
  Add blocking layer for C bindings (#3278)
  feat(services/redb): change blocking_x in async_x call to tokio::task::blocking_spawn (#3276)
  refactor(bindings/C): Implement error with error message (#3250)
  test(binding/java): grouping tests with tag (#3271)
  reafctor: Polish error process code for mysql/postgresql services (#3269)
  feat(service/d1): Support d1 for opendal (#3248)
  feat: Add edge test cases for OpenDAL Core (#3274)
  ci: Passing GITHUB_TOKEN to avoid rate limit (#3272)
  ci: add a new test case for the disk is full (#3079)
  ci: Only fetch origin while in pull request (#3268)
  ...
Comment thread bindings/nodejs/package.json Outdated
Comment thread bindings/nodejs/tests/test_service.test.js Outdated
Comment thread bindings/nodejs/tests/test_service.test.js Outdated
Comment thread bindings/nodejs/tests/test_service.test.js Outdated
Comment thread bindings/nodejs/package.json Outdated
Comment thread bindings/nodejs/generated.js
Comment thread bindings/nodejs/tests/test_service.test.js Outdated
@eryue0220
Copy link
Copy Markdown
Contributor Author

Currently, the scheme and config will be loaded from the .env configuration.

Comment thread bindings/nodejs/tests/suites/sync.suite.mjs Outdated
Copy link
Copy Markdown
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Sorry, just found that I didn't submit my review.

Comment thread bindings/nodejs/tests/suites/index.mjs
@eryue0220 eryue0220 marked this pull request as draft November 1, 2023 01:47
@eryue0220 eryue0220 marked this pull request as ready for review November 1, 2023 02:19
@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented Nov 1, 2023

Hi, @eryue0220, this PR is almost set. Can I directly push some additional changes to this PR to align it with the modifications we've made for the main branch?

@eryue0220
Copy link
Copy Markdown
Contributor Author

most set. Can I directly push some additional changes to this PR to align it with the modifications we've made for the main bra

Yeah, for sure

Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Comment thread bindings/nodejs/tests/utils.mjs Outdated
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented Nov 1, 2023

This feature still have a lot work to do:

  • implement capability
  • implement OPENDAL_DISABLE_RAMDOM_ROOT

But this PR is large enough that I removed not passing test cases to make it mergable first. We can improve it in the coming PRs.

@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented Nov 1, 2023

Apologies for the length of this PR, it has taken @eryue0220 more time than anticipated.

I learnt some lessons from this PR:

  • Don't start work too soon: It's better to discuss about the details first especially for complex work and new contributors like this one.
  • Don't push work too large: It's better to split work in different PRs instead of a huge one.

I believe that we could make the exprience much better if we:

  • Implement the test framework first
  • Integrate with the simplest test case to make sure it works
  • Expand to all services
  • Remove old tests

Regardless, thank you @eryue0220 for your patience and hard work! I promise that we will make the exprience better ❤️

Copy link
Copy Markdown
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

THank you @eryue0220!

@Xuanwo Xuanwo merged commit 8618b57 into apache:main Nov 1, 2023
@eryue0220
Copy link
Copy Markdown
Contributor Author

eryue0220 commented Nov 1, 2023

THank you @eryue0220!

Welcome. I appreciated the opendal WG and your effort. I also really learned more from this contribution experience. Finally, this is my first Apache PR, achieve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

releases-note/feat The PR implements a new feature or has a title that begins with "feat"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants