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

NC | Fix test_nc_upgrade_manager cleanup #8636

Merged

Conversation

romayalon
Copy link
Contributor

@romayalon romayalon commented Dec 30, 2024

Explain the changes

  1. Fixed a bug in the mode of some paths created during the test - changed permissions from 777 to 0o777.
  2. Changed to the 0.0.9 nc upgrade scripts directory a '0.0.9-test-nc-upgrade-manager' so it'll be extra understood this is not a real nc_upgrade_scripts dir but a mock dir for tests.
  3. Added a missing call to await fs_utils.folder_delete(default_upgrade_script_dir_version_path); in order to clean-up /Users/{user_name}/github/noobaa-core/src/upgrade/nc_upgrade_scripts/0.0.9-test-nc-upgrade-manager.

Issues: Fixed #xxx / Gap #xxx

  1. Fixed partially NC | Online upgrade improvements #8586

Testing Instructions:

  1. Run -
sudo jest --testRegex=jest_tests/test_nc_upgrade_manager.test.js --t "nc upgrade manager - _run_nc_upgrade_scripts` 
sudo ls -la  /Users/{user_name}/github/noobaa-core/src/upgrade/nc_upgrade_scripts/

Check that /Users/{user_name}/github/noobaa-core/src/upgrade/nc_upgrade_scripts/0.0.9-test-nc-upgrade-manager does not exist.

  • Doc added/updated
  • Tests added

Copy link
Contributor

@shirady shirady left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com>
@romayalon romayalon force-pushed the romy-fix-test-nc-upgrade-manager-test branch from ce70c34 to 7353ce9 Compare January 2, 2025 08:48
@romayalon romayalon merged commit eec9e18 into noobaa:master Jan 2, 2025
11 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