From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id 713F91FF173
	for <inbox@lore.proxmox.com>; Wed, 31 Jul 2024 11:47:11 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 19036AF4F;
	Wed, 31 Jul 2024 11:47:14 +0200 (CEST)
Message-ID: <a5f144a7-ce9a-4f18-8b9f-8613192c3691@proxmox.com>
Date: Wed, 31 Jul 2024 11:47:10 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>, Dominik Csapak <d.csapak@proxmox.com>
References: <20240731093604.1315088-1-d.csapak@proxmox.com>
 <20240731093604.1315088-3-d.csapak@proxmox.com>
Content-Language: de-AT, en-US
From: Lukas Wagner <l.wagner@proxmox.com>
In-Reply-To: <20240731093604.1315088-3-d.csapak@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.008 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pbs-devel] [PATCH proxmox-backup v2 2/4] datastore: test
 DataBlob encode/decode roundtrip
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>

On  2024-07-31 11:36, Dominik Csapak wrote:
> so that we can be sure we can decode an encoded blob again
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> new in v2
>  pbs-datastore/src/data_blob.rs | 66 ++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/pbs-datastore/src/data_blob.rs b/pbs-datastore/src/data_blob.rs
> index a7a55fb7..8715afef 100644
> --- a/pbs-datastore/src/data_blob.rs
> +++ b/pbs-datastore/src/data_blob.rs
> @@ -562,3 +562,69 @@ impl<'a, 'b> DataChunkBuilder<'a, 'b> {
>          chunk_builder.build()
>      }
>  }
> +
> +#[cfg(test)]
> +mod test {
> +    use pbs_tools::crypt_config::CryptConfig;
> +
> +    use super::DataChunkBuilder;
> +
> +    const TEST_DATA_LEN: usize = 50;
> +
> +    #[test]
> +    fn test_data_blob_builder() {
> +        let mut data = Vec::with_capacity(TEST_DATA_LEN);
> +        for i in 0..TEST_DATA_LEN / 10 {
> +            for _ in 0..10 {
> +                data.push(i as u8);
> +            }
> +        }
> +
> +        // unencrypted, uncompressed
> +        let (chunk, digest) = DataChunkBuilder::new(&data)
> +            .compress(false)
> +            .build()
> +            .expect("could not create unencrypted, uncompressed chunk");
> +
> +        let data_decoded = chunk
> +            .decode(None, Some(&digest))
> +            .expect("cannot decode unencrypted, uncompressed chunk");
> +        assert_eq!(data, data_decoded);
> +
> +        // unencrypted, compressed
> +        let (chunk, digest) = DataChunkBuilder::new(&data)
> +            .compress(true)
> +            .build()
> +            .expect("could not create unencrypted, compressed chunk");
> +
> +        let data_decoded = chunk
> +            .decode(None, Some(&digest))
> +            .expect("cannot decode unencrypted, compressed chunk");
> +        assert_eq!(data, data_decoded);
> +
> +        // encrypted, uncompressed
> +        let crypt_config = CryptConfig::new([9; 32]).expect("could not create crypt config");
> +        let (chunk, digest) = DataChunkBuilder::new(&data)
> +            .compress(false)
> +            .crypt_config(&crypt_config)
> +            .build()
> +            .expect("could not create encrypted, uncompressed chunk");
> +
> +        let data_decoded = chunk
> +            .decode(Some(&crypt_config), Some(&digest))
> +            .expect("cannot decode encrypted, uncompressed chunk");
> +        assert_eq!(data, data_decoded);
> +
> +        // encrypted, compressed
> +        let (chunk, digest) = DataChunkBuilder::new(&data)
> +            .compress(true)
> +            .crypt_config(&crypt_config)
> +            .build()
> +            .expect("could not create encrypted, compressed chunk");
> +
> +        let data_decoded = chunk
> +            .decode(Some(&crypt_config), Some(&digest))
> +            .expect("cannot decode encrypted, compressed chunk");
> +        assert_eq!(data, data_decoded);
> +    }
> +}

IMO it would be nicer to move the (sub) testcases 
(e.g. 'encrypted, compressed', 'encrypted, uncompressed') to individual test cases,
transforming the info from the comments into the test name, for instance:

  #[test]
  fn test_encrypted_uncompressed() { ... }

(the fact that you test the data_blob module is clear from the full test name, which includes
the module prefix - so I think you can drop it from the test name)

IMO that is quite nice when at some point we end up with a failing assertion, because
you see exactly from the name of the failing test what scenario did not work.

What's your take on this?

-- 
- Lukas


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel