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

NSFS | NC | NC Coretests changes - PR 2/3 #7720

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

romayalon
Copy link
Contributor

WIP - This PR is dependent on merging #7719

Explain the changes

  1. Makefile - added run-nc-tests makefile rule for running nc_index.js tests file.
  2. test_utils.js - added a helper function for get_coretest_path() that chooses coretest/nc_coretest path by process.env.NC_CORETEST.
  3. nc_coretest.js - config_dir, storage, and other setups and starting nsfs.js process, added mapping between rpc_client calls and manage_nsfs commands or other mocks.
  4. nc_index.js - added NSFS relevant test files.
  5. test_bucketspace.js - added skips by process.env.NC_CORETEST of non-relevant tests, and different error_codes.

Issues: Fixed #xxx / Gap #xxx

  1. Gap - nc_index.js contains commented test files that needs to be added as well.
  2. github action for running nc_index.js

Testing Instructions:

  • Doc added/updated
  • Tests added

@romayalon romayalon requested a review from naveenpaul1 January 15, 2024 09:17
@romayalon romayalon marked this pull request as draft January 15, 2024 09:19
Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com>
@romayalon romayalon marked this pull request as ready for review January 16, 2024 09:21
Copy link
Contributor

@naveenpaul1 naveenpaul1 left a comment

Choose a reason for hiding this comment

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

A comman doubt : - How this different from the normal test?

* @param {object} options
* @returns {Promise<void>}
*/
async function update_account_s3_access_manage(options) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@romayalon are we validating this account update anywhere else? just a doubt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

on the tests, this function equal to the rpc function - update_account_s3_access(), the difference here is that on Non containerized version we don't have RPC server so we do the update by running manage NSFS cli calls instead and return rpc like response.

@romayalon
Copy link
Contributor Author

@naveenpaul1 The regular Coretest runs noobaa endpoint (and other components of noobaa core) and then a test can run full s3 flow.
NC Coretest runs nsfs.js instead of running the endpoint and by that, we run the NC code. Then a test can run full s3 flow on NC environment. I hope it makes it clearer, let me know if there are more questions.

Copy link
Contributor

@naveenpaul1 naveenpaul1 left a comment

Choose a reason for hiding this comment

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

LGTM

@romayalon romayalon merged commit bf0a557 into noobaa:master Jan 17, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants