[tests-only][full-ci] add tests for post processing command for different steps #11607

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

Open

nabim777 wants to merge 2 commits into master from tests/post-processing-resume

Member

nabim777 commented Aug 15, 2025 •

edited

Loading

Description

This PR adds a test for postprocessing command to resume on finished and virusscan steps defined failed uploads.

ocis postprocessing resume                # Resumes all uploads where postprocessing is finished, but upload is not finished.
ocis postprocessing resume -s "finished"  # Equivalent to the above.
ocis postprocessing resume -s "virusscan" # Resume all uploads currently in virusscan step.

For more information about this command 👉 click here

Related Issue

part of - [QA][CLI] Add cli tests coverage for post processing service #11273

Motivation and Context

Screenshots (if appropriate):

Types of changes

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 change)
Technical debt
Tests only (no source changes)

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

nabim777 mentioned this pull request

[QA][CLI] Add cli tests coverage for post processing service #11273

Open

5 tasks

nabim777 force-pushed the tests/post-processing-resume branch from da6ed82 to d9deb78 Compare

August 18, 2025 04:24

nabim777 self-assigned this

nabim777 added the QA:team label

nabim777 force-pushed the tests/post-processing-resume branch 2 times, most recently from 2245bce to d1d300b Compare

August 19, 2025 09:26

nabim777 commented

View reviewed changes

tests/acceptance/bootstrap/CliContext.php

               		// $responseBody["message"] contains a message info with the array of output json of the upload sessions command
               		// Example Output: "INFO memory is not limited, skipping package=github.com/KimMachineGun/automemlimit/memlimit [{<output-json>}]"
               		// So, only extracting the array of output json from the message
-              		\preg_match('/(\[.*\])/', $responseBody["message"], $matches);
-              		return \json_decode($matches[1], null, 512, JSON_THROW_ON_ERROR);
+              		\preg_match('/(\[.*?\])/', $responseBody["message"], $matches);

Member Author

nabim777 Aug 19, 2025

- \preg_match('/(\[.*\])/', $responseBody["message"], $matches);
+ \preg_match('/(\[.*?\])/', $responseBody["message"], $matches);

making regex non-greedy

Member

saw-jan Aug 26, 2025

but why was it required?

Member Author

nabim777 Sep 1, 2025 •

edited

Loading

I think it's not needed for current scenario, so I will leave it as is (no changes)

nabim777 commented

View reviewed changes

tests/acceptance/bootstrap/CliContext.php Outdated Show resolved Hide resolved

nabim777 marked this pull request as ready for review

August 19, 2025 09:33

nabim777 requested review from saw-jan, PrajwolAmatya, anon-pradip, prashant-gurung899 and nirajacharya2

August 19, 2025 09:33

nabim777 force-pushed the tests/post-processing-resume branch from d1d300b to 087234c Compare

August 19, 2025 09:41

prashant-gurung899 reviewed

View reviewed changes

tests/acceptance/bootstrap/CliContext.php Outdated Show resolved Hide resolved

nabim777 requested a review from Ashim-Stha

August 22, 2025 03:41


          add tests for post processing command for different steps

d8cf3c0

Signed-off-by: nabim777 <[email protected]>

nabim777 force-pushed the tests/post-processing-resume branch from 087234c to 418ddc0 Compare

August 25, 2025 08:58

nabim777 requested a review from prashant-gurung899

August 25, 2025 08:58


          refactor for adding not upload tests

d6eab5f

Signed-off-by: nabim777 <[email protected]>

nabim777 force-pushed the tests/post-processing-resume branch from 418ddc0 to d6eab5f Compare

August 25, 2025 08:59

sonarqubecloud bot commented Aug 25, 2025

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

PrajwolAmatya reviewed

View reviewed changes

tests/acceptance/bootstrap/CliContext.php

               		$this->featureContext->setResponse(CliHelper::runCommand($body));
               	}
+              	/**
+              	 * @When /^the administrator resumes all the uploads session using the post processing command$/

Contributor

PrajwolAmatya Aug 26, 2025

Suggested change

      
            	 * @When /^the administrator resumes all the uploads session using the post processing command$/
          
            	 * @When the administrator resumes all the uploads session using the post processing command

saw-jan reviewed

View reviewed changes

tests/acceptance/features/cliCommands/uploadSessions.feature

Comment on lines +328 to +331

+                Scenario: resume virus file upload failed on virus scan step using postprocessing command
+                  Given the following configs have been set:
+                    | config                           | value           |
+                    | POSTPROCESSING_STEPS             | delay,virusscan |

Member

saw-jan Aug 26, 2025

The above scenario should cover this as well

tests/acceptance/features/cliCommands/uploadSessions.feature

Comment on lines +301 to +313

+                    | POSTPROCESSING_STEPS             | delay,virusscan |
+                    | ANTIVIRUS_INFECTED_FILE_HANDLING | abort           |
+                    | POSTPROCESSING_DELAY             | 10s             |
+                  And user "Alice" has created a new TUS resource in the space "Personal" with the following headers:
+                    | Upload-Length   | 10                        |
+                    #    dGV4dEZpbGUudHh0 is the base64 encode of textFile.txt
+                    | Upload-Metadata | filename dGV4dEZpbGUudHh0 |
+                    | Tus-Resumable   | 1.0.0                     |
+                  And user "Alice" has uploaded file with checksum "SHA1 8cb2237d0679ca88db6464eac60da96345513964" to the last created TUS Location with offset "0" and content "12345" via TUS inside of the space "Personal" using the WebDAV API
+                  And user "Alice" has uploaded file "filesForUpload/filesWithVirus/eicar.com" to "/virusFile.txt"
+                  And user "Alice" has uploaded file with content "uploaded content" to "file.txt"
+                  And the administrator has waited for "1" seconds
+                  And the administrator has stopped the server

Member

saw-jan Aug 26, 2025

with this flow, the file upload doesn't fail on virusscan, it fails on delay step

tests/acceptance/features/cliCommands/uploadSessions.feature

@@ @@ -245,3 +245,107 @@ Feature: List upload sessions via CLI command @@
                   When the administrator waits for "3" seconds
                   Then for user "Alice" file "file2.txt" of space "Personal" should be in postprocessing
                   And the content of file "file1.txt" for user "Alice" should be "uploaded content"
+                Scenario: resume all failed upload but not for unfinished upload using postprocessing command

Member

saw-jan Aug 26, 2025

finding it difficult to understand the title

Suggested change

      
              Scenario: resume all failed upload but not for unfinished upload using postprocessing command
          
              Scenario: resume all upload sessions containing unfinished upload using postprocessing command

tests/acceptance/features/cliCommands/uploadSessions.feature

+                  And the administrator has waited for "1" seconds
+                  And the administrator has stopped the server
+                  And the administrator has started the server
+                  When the administrator resumes all uploads session in finished step using post processing command

Member

saw-jan Aug 26, 2025

what constitutes a finished step? the test flow is same creating failed uploads

saw-jan reviewed

View reviewed changes

tests/acceptance/features/cliCommands/uploadSessions.feature

+                  And the administrator has waited for "1" seconds
+                  And the administrator has stopped the server
+                  And the administrator has started the server
+                  When the administrator resumes all the uploads session using the post processing command

Member

saw-jan Aug 27, 2025

Suggested change

      
                When the administrator resumes all the uploads session using the post processing command
          
                When the administrator resumes all the upload sessions using the postprocessing command

saw-jan reviewed

View reviewed changes

tests/acceptance/features/cliCommands/uploadSessions.feature

+                  And the administrator has waited for "1" seconds
+                  And the administrator has stopped the server
+                  And the administrator has started the server
+                  When the administrator resumes all uploads session in finished step using post processing command

Member

saw-jan Aug 27, 2025

Suggested change

      
                When the administrator resumes all uploads session in finished step using post processing command
          
                When the administrator resumes all upload sessions in finished step using postprocessing command

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

Reviewers

saw-jan saw-jan left review comments

PrajwolAmatya PrajwolAmatya left review comments

anon-pradip Awaiting requested review from anon-pradip

nirajacharya2 Awaiting requested review from nirajacharya2

Ashim-Stha Awaiting requested review from Ashim-Stha

prashant-gurung899 Awaiting requested review from prashant-gurung899

At least 1 approving review is required to merge this pull request.

Labels