Adding integration tests to checksums by SergioBertolinSG · Pull Request #27345 · owncloud/core

SergioBertolinSG · 2017-03-09T12:49:02Z

Requires #27097

mention-bot · 2017-03-09T12:49:03Z

@SergioBertolinSG, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975, @PVince81 and @bartv2 to be potential reviewers.

SergioBertolinSG · 2017-03-09T14:25:24Z

These tests are removed and will be added when copy returns the checksum. Currently they fail.

They require #26584 to be done before.

Scenario: Copying a file with checksum should return the checksum in the propfind
    Given using old dav path
    And user "user0" exists
    And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a"
    When User "user0" copies file "/myChecksumFile.txt" to "/myChecksumFileCopy.txt"
    And user "user0" request the checksum of "/myChecksumFileCopy.txt" via propfind
    Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f"

  Scenario: Copying file with checksum should return the checksum in the download header
    Given using old dav path
    And user "user0" exists
    And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a"
    When User "user0" copies file "/myChecksumFile.txt" to "/myChecksumFileCopy.txt"
    And user "user0" downloads the file "/myChecksumFileCopy.txt"
    Then The header checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f"

SergioBertolinSG · 2017-03-10T09:07:24Z

These tests where removed from webdav-related.feature and needs to be added here:

        Scenario: Upload chunked file where checksum does not match
                Given using new dav path
                And user "user0" exists
                And user "user0" creates a new chunking upload with id "chunking-42"
                And user "user0" uploads new chunk file "2" with "BBBBB" to id "chunking-42"
                And user "user0" uploads new chunk file "3" with "CCCCC" to id "chunking-42" with checksum "SHA1:f005ba11"
                And user "user0" uploads new chunk file "1" with "AAAAA" to id "chunking-42"
                And user "user0" moves new chunk file with id "chunking-42" to "/myChunkedFile.txt"
                Then the HTTP status code should be "400"

        Scenario: Upload a file where checksum does not match
                Given user "user0" exists
                And file "/chksumtst.txt"  does not exist for user "user0"
                And user "user0" uploads file with checksum "SHA1:f005ba11" and content "Some Text" to "/chksumtst.txt"
                Then the HTTP status code should be "400"

        Scenario: Upload a file where checksum does match
                Given user "user0" exists
                And file "/chksumtst.txt"  does not exist for user "user0"
                And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/chksumtst.txt"
                Then the HTTP status code should be "201"

        Scenario: Uploaded file should have the same checksum when downloaded
                Given user "user0" exists
                And file "/chksumtst.txt"  does not exist for user "user0"
                And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/chksumtst.txt"
                When Downloading file "/chksumtst.txt" as "user0"
                Then The following headers should be set
                        | OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 |

PVince81 · 2017-03-10T10:25:09Z

10:47:19 Using database oc_autotest3
10:47:19 Setup environment for mysqlmb4 testing on local storage ...
10:47:19 Fire up the mysql docker
10:47:20 Waiting for MySQL(utf8mb4) initialisation ...
10:57:22 ........................................................................................................................................................................................................
10:57:22 [ERROR] Waited 600 seconds, no response
10:57:22 Kill the docker 9b7dee09bda11058a69097640488fc4b31fde0e70c0f8a395416367bdce21356
10:57:22 9b7dee09bda11058a69097640488fc4b31fde0e70c0f8a395416367bdce21356
10:57:22 9b7dee09bda11058a69097640488fc4b31fde0e70c0f8a395416367bdce21356
10:57:22 Makefile:177: recipe for target 'test-php' failed

PVince81 · 2017-03-13T15:03:55Z

+	public function userCopiedFileTo($user, $source, $destination) {
+		$client = new Client();
+		$request = $client->createRequest(
 			'MOVE',


copy does a move ? 🤕

Yes, thats why I removed copy tests #27345 (comment)

I'll modify this function.

PVince81 · 2017-03-13T15:04:10Z

-	public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination, $checksum)
-	{
+	public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination, $checksum) {
+		//$client = new Client();


commented out code

PVince81 · 2017-03-13T15:04:53Z

+    And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
+    And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
+    When user "user0" downloads the file "/myChecksumFile.txt"
+    Then The header checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8"


also check the MD5 ? I see it's passed at upload time ?

Do you mean add another check there?

Then The header checksum should match "MD5:45a72715acdd5019c5be30bdbb75233e"

This fails.

Yes. Why does this fail ? Is the MD5 wrong ? Please investigate.

Or alternatively remove the MD5 from this test as you are testing the computed checksum, not the passed checksum.

Then make a separate test that checks if the stored checksum as passed from the client works.

Well it fails because the sha1 works:

Expected MD5:45a72715acdd5019c5be30bdbb75233e, got SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8

Don't we return multiple checksums anyway ? Maybe you need to adjust the checking code to pick the matching type and compare that one ?

No, just one:

[oc-checksum] => Array
│ (
│ [0] => SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8
│ )

But aren't we computing three different checksum types ? Why only return one then ?

cc @IljaN

@PVince81
Yeah we are computing three different sums. But I decided to return one. Actually I am not sure anymore what my reasoning was. I think because existing tests expected one sum in the webdav body and I was afraid to break some protocols.

I suppose we could easily change that to have multiple checksums (space seperated) returned on a PROPFIND.

@SergioBertolinSG PROPFIND nows returns all checksums, I also modified the tests in checksums.feature accordingly

@IljaN Great, thanks.

(It is really needed to compute three checksums? I guess you already discuss that, but seems weird)

SergioBertolinSG · 2017-03-17T07:54:56Z

👍 for last changes

phil-davis · 2018-02-01T14:25:19Z

+    And user "user0" exists
+    And file "prueba_cksum.txt" with text "Test file for checksums" is created in local storage
+    When user "user0" downloads the file "/local_storage/prueba_cksum.txt"
+    When user "user0" downloads the file "/local_storage/prueba_cksum.txt"


@SergioBertolinSG I am doing some refactoring and noticed that this line is duplicated.
When I remove 1 copy, the check for header checksum below fails (it cannot find the file).
Do you remember why this download needs to be done twice?

@phil-davis It might be because checksum is computed on first download for files which have none.

Ah yes, thanks. I see that the previous test step creates the file directly on the local storage (bypassing ownCloud). So the first download triggers the checksum calculation.
I will make a comment in the feature file while refactoring so the next person does not have to research this.

lock · 2019-08-01T08:02:12Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

SergioBertolinSG added 2 - Developing dev:integration-tests labels Mar 9, 2017

SergioBertolinSG added this to the 10.0 milestone Mar 9, 2017

SergioBertolinSG self-assigned this Mar 9, 2017

SergioBertolinSG requested a review from PVince81 March 9, 2017 12:49

SergioBertolinSG force-pushed the inttest-checksums-from-26655_ComputeChecksum2 branch from e545b5f to 7b5fc8b Compare March 10, 2017 09:44

SergioBertolinSG added 3 commits March 10, 2017 11:44

Removed checksums context and integrate it with the rest

cee9935

Adapted tests to use new dav endpoint

a2257f0

Added Ilja's tests here

2ba31b6

SergioBertolinSG force-pushed the inttest-checksums-from-26655_ComputeChecksum2 branch from 7b5fc8b to 2ba31b6 Compare March 10, 2017 11:44

SergioBertolinSG added 3 - To Review and removed 2 - Developing labels Mar 13, 2017

SergioBertolinSG mentioned this pull request Mar 13, 2017

Tests for checksums owncloud/QA#375

Closed

9 tasks

PVince81 reviewed Mar 13, 2017

View reviewed changes

SergioBertolinSG and others added 2 commits March 14, 2017 09:35

Copy will copy and not move

d3dd0c8

Webdav returns all checksums + integration tests

014530b

PVince81 merged commit fe3e383 into master Mar 17, 2017

PVince81 deleted the inttest-checksums-from-26655_ComputeChecksum2 branch March 17, 2017 07:57

MorrisJobke mentioned this pull request Mar 24, 2017

Server side checksum verification nextcloud/server#4023

Closed

phil-davis reviewed Feb 1, 2018

View reviewed changes

lock Bot locked as resolved and limited conversation to collaborators Aug 1, 2019

Uh oh!

Conversation

SergioBertolinSG commented Mar 9, 2017

Uh oh!

mention-bot commented Mar 9, 2017

Uh oh!

SergioBertolinSG commented Mar 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SergioBertolinSG commented Mar 10, 2017

Uh oh!

PVince81 commented Mar 10, 2017

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!

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!

IljaN Mar 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SergioBertolinSG Mar 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SergioBertolinSG commented Mar 17, 2017

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!

lock Bot commented Aug 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SergioBertolinSG commented Mar 9, 2017 •

edited

Loading

IljaN Mar 14, 2017 •

edited

Loading

SergioBertolinSG Mar 16, 2017 •

edited

Loading