Cli-testing migration : remove small dependencies by SylvainSenechal · Pull Request #2548 · scality/Zenko

SylvainSenechal · 2026-10-05T08:54:28Z

Moving from cli-testing directly into this codebase :

a few constants
utility functions

Upcoming work for cli-testing, 2 more tickets :

Kubernetes client
Sdk commands

bert-e · 2026-10-05T08:54:32Z

Hello sylvainsenechal,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/bypass_source_branch_lineage`	Bypass the cross-branch contamination check	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request.
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2026-10-05T12:25:38Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

francoisferrand · 2026-10-07T15:16:38Z

-        storageAccountOwner: process.env.STORAGE_ACCOUNT_OWNER || 'storage_account_owner',
-        dataConsumer: process.env.DATA_CONSUMER || 'data_consumer',
-        dataAccessor: process.env.DATA_ACCESSOR || 'data_accessor',
+        account: requireEnv('ACCOUNT'),


should we not use the values from default ?

otherwise the values -like ACCOUNT_NAME ?- there should be dropped (note: having stuff defined in a single place is better indeed, but not sure it is so good to require everything to come from env : this will make it harder to eventually drop the startup bash script and run/debug simply with yarn/...)

honestly the state of this file is still pretty bad
Ctst is mixing env var var with proper cucumber parameters, so eventually it would be nice to just use one of the two, and have constants defined in the tests directly if its not needed by the setup files. Here I was just doing quick cleanup but its not super important for the goal of this pr

I just think for later its better to have the same behavior for all these var so that its easier to spot the utilisation of env var when doing a refactoring, if we use default value we might missed it

my point here is that you added default values for ACCONT in constants.ts : so either it should be removed from the constants, or it should be used... but I don't see either: it is removed here, and I don't see where we use it.

on the scope of PR :

not clear how this is "quick cleanup", and completely unrelated to the "remove small dependencies" goal which already touches so many lines/files

there is something do be done abut config - yes. But the proper way would be that we consistently use properly typed data, and do the mapping from env (or default) just once. Like build a cucumber World once from env and default, for exemple. But until we define (and start working) where we are, just rewriting the code one way or another looks like churn...

I'm removing the last 2 commits, it's not important to deal with these now we can do it later.
I just wanna remove imports from cli-testing on this pr

francoisferrand

LGTM, but I don't really see the point: it is not better or worse, just different
But I don't understand the move of constants.ts : we we define such defaults, why not use them (in requireEnv) ? Here you just remove all defaults, yet we know that we want to move most of the defaults in the code anyway: so changing the code while knowing we will still need to change again...

Issue: ZENKO-5372

SylvainSenechal · 2026-10-23T13:31:55Z

    accountKey: string;
 };

-function requireEnv(name: string): string {


this was double defined

SylvainSenechal · 2026-10-23T13:39:27Z

Btw I agree it's not a super useful pr, i'm trying to remove the dependency on cli-testing, I'm breaking it down a bit starting with this easy one because there are 2 more and it will be a lot of code in a single pr

maeldonn

LGTM

maeldonn · 2026-10-23T13:43:29Z

 import { createBucketWithConfiguration, putObject } from '../steps/utils/utils';
 import { createJobAndWaitForCompletion } from '../steps/utils/kubernetes';

+export function sleep(ms: number): Promise<void> {


Node already ships this: timers/promises exposes a promisified setTimeout.

Suggested change

export function sleep(ms: number): Promise<void> {

export { setTimeout as sleep } from 'timers/promises';

maeldonn · 2026-10-23T13:50:37Z

+    return new Promise(resolve => setTimeout(resolve, ms));
+}
+
+export function randomString(): string {


crypto can do this in one line:

import { createHash, randomBytes } from 'crypto'; export function randomString(): string { return randomBytes(5).toString('hex'); }

Output becomes lowercase hex (0-9a-f) instead of mixed-case alphanumeric. Fine here: names stay 10 chars, collisions stay negligible for tests 🤷‍♂️

yeah thank you

maeldonn · 2026-10-23T13:55:27Z

                    const accessKeys = await SuperAdmin.generateAccountAccessKey({ accountName });
-                    if (!Utils.isAccessKeys(accessKeys)) {
+                    if (!isAccessKeys(accessKeys)) {
                        throw new Error('Failed to generate account access keys for site ${siteKey}');


Unrelated to this commit: these error strings use single quotes, so ${siteKey} isn't interpolated.

…Keys helpers Issue: ZENKO-5372

Issue: ZENKO-5372

SylvainSenechal · 2026-10-23T15:37:35Z

/approve

bert-e · 2026-10-23T16:20:57Z

Build failed

The build for commit did not succeed in branch improvement/ZENKO-5372

The following options are set: approve

SylvainSenechal · 2026-10-23T16:35:52Z

/approve

bert-e · 2026-10-23T22:34:37Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/2.16

The following branches have NOT changed:

development/2.10
development/2.11
development/2.12
development/2.13
development/2.14
development/2.15
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

This pull request did not target the following hotfix branch(es) so they
were left untouched:

hotfix/2.13.5

Please check the status of the associated issue ZENKO-5372.

Goodbye sylvainsenechal.

The following options are set: approve

SylvainSenechal mentioned this pull request Oct 5, 2026

Cli-testing migration : remove small dependencies #2547

Closed

SylvainSenechal marked this pull request as ready for review October 5, 2026 11:08

SylvainSenechal requested review from a team, DarkIsDude and francoisferrand October 5, 2026 11:08

SylvainSenechal added this pull request to stack #2550 October 5, 2026 12:07

SylvainSenechal force-pushed the improvement/ZENKO-5372 branch from 5931178 to 171a3f7 Compare October 5, 2026 12:17

DarkIsDude approved these changes Oct 5, 2026

View reviewed changes

scality deleted a comment from bert-e Oct 5, 2026

francoisferrand reviewed Oct 7, 2026

View reviewed changes

SylvainSenechal requested a review from francoisferrand October 9, 2026 18:04

francoisferrand reviewed Oct 13, 2026

View reviewed changes

Move constants from cli-testing into the local ctst constants file

98026ec

Issue: ZENKO-5372

SylvainSenechal force-pushed the improvement/ZENKO-5372 branch 2 times, most recently from ba7b129 to 087ac65 Compare October 23, 2026 13:31

SylvainSenechal commented Oct 23, 2026

View reviewed changes

scality deleted a comment from bert-e Oct 23, 2026

maeldonn self-requested a review October 23, 2026 13:38

scality deleted a comment from bert-e Oct 23, 2026

maeldonn approved these changes Oct 23, 2026

View reviewed changes

SylvainSenechal added 4 commits October 23, 2026 16:06

Replace cli-testing Utils with local sleep, randomString and isAccess…

3d8d3c0

…Keys helpers Issue: ZENKO-5372

Fix isAccessKeys always returning true

39db8c9

Issue: ZENKO-5372

Deduplicate requireEnv into clients/env.ts

42df3fe

Issue: ZENKO-5372

Fix uninterpolated template strings in account access key errors

4ca47ad

Issue: ZENKO-5372

SylvainSenechal force-pushed the improvement/ZENKO-5372 branch from 087ac65 to 4ca47ad Compare October 23, 2026 14:10

bert-e merged commit 4ca47ad into development/2.16 Oct 23, 2026
92 of 96 checks passed

bert-e deleted the improvement/ZENKO-5372 branch October 23, 2026 22:34

	export function sleep(ms: number): Promise<void> {
	export { setTimeout as sleep } from 'timers/promises';

Conversation

SylvainSenechal commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bert-e commented Oct 5, 2026

Hello sylvainsenechal,

Uh oh!

bert-e commented Oct 5, 2026

Waiting for approval

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

francoisferrand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SylvainSenechal commented Oct 23, 2026

Uh oh!

maeldonn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SylvainSenechal commented Oct 23, 2026

Uh oh!

bert-e commented Oct 23, 2026

Build failed

Uh oh!

SylvainSenechal commented Oct 23, 2026

Uh oh!

bert-e commented Oct 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SylvainSenechal commented Oct 5, 2026 •

edited

Loading