Question: Is there a better way to do unit test

[Xunop]

Hello!
I successfully built imapsync using the PKGBUILD (Arch Linux package build description file). You can find the PKGBUILD file at this link: https://gitlab.archlinux.org/archlinux/packaging/packages/imapsync/-/blob/main/PKGBUILD.

The build process proceeded smoothly; however, I encountered an issue during the check function's execution of the make test command, which will run the tests.sh script. This script relies on a running IMAP server with several accounts, and some of its test functions require access to email secrets. Unfortunately, this setup causes the tests to fail.

Despite the failure, the script continues its execution. Specifically, the ll_env_password function attempts to test a password sent via environment variable. The environment variable is set to run cat ../../var/pass/secret.tata or secret.titi, but this fails. As a result, imapsync prompts the user to enter the password in the terminal. This manual input maybe not very good, especially on a remote server. There are a number of places in cat ../../var/pass/secret.tata where secret is used, so maybe we can skip those tests? Now, I just remove ll_env_password from mandatory_tests.

[gilleslamiral]

Forget tests.sh it's functional tests for me. I should do a "work for everyone" mode but I'm lazy

./imapsync --tests
./imapsync --testslive
./imapsync --testslive6

are faster and cover many things.

[Xunop]

Hello @gilleslamiral, I apologize for the delayed response. I have used the commands you provided and discovered that some tests failed. I have fixed the issues after reviewing the code over the last few days(I'm not familiar with perl:( ). However, it appears that the domain in testslive6, namely ks6ipv6.lamiral.info, is no longer in use.

Can I create a pull request to fix these failed tests?

[gilleslamiral]

If you spend time on imapsync, work on this one instead:
https://imapsync.lamiral.info/imapsync

[Xunop]

If you spend time on imapsync, work on this one instead: https://imapsync.lamiral.info/imapsync

Great! I will use this version for unit test.

[Xunop]

Hi @gilleslamiral , I'm use the latest version of imapsync and ran ./imapsync --tests. However, some tests failed:

nb 2090 - tests_cgidir: {  } => /var/tmp/imapsync_cgi/385d...
nb 2092 - tests_cgidir: Net::Server::HTTP + Docker => /var/tmp/imapsync_cgi/385d...

Upon reviewing the code, it seem that the hashsynclocal function generates a different hash due to different hashkey. Therefore, I passed a stable hashkey to the hashsynclocal function in cgidir function, resulting in a stable hash.

I made the following changes, and the tests_cgidir passed:

diff --git a/imapsync b/imapsync
index f8cf1e0..94dbb07 100644
--- a/imapsync
+++ b/imapsync
@@ -4239,14 +4239,14 @@ sub tests_cgidir
                 else
                 {
                         my $mysync = {  } ;
-                        is( '/var/tmp/imapsync_cgi/385d7a4d8d428d7aa2b57c8982629e2bd67698ed', cgidir( $mysync ),  'tests_cgidir: {  } => /var/tmp/imapsync_cgi/385d...' ) ;
+                        is( '/var/tmp/imapsync_cgi/70348345eb2460c97129e7a3f8ffb8842e90c4e4', cgidir( $mysync ),  'tests_cgidir: {  } => /var/tmp/imapsync_cgi/7034...' ) ;
 
                         {
                                 local $ENV{ NET_SERVER_SOFTWARE } = 'Net::Server::HTTP' ;
                                 is( '.', cgidir( $mysync ),  'tests_cgidir: Net::Server::HTTP => .' ) ;
                 
                                 $mysync->{ dockercontext } = 1 ;
-                                is( '/var/tmp/imapsync_cgi/385d7a4d8d428d7aa2b57c8982629e2bd67698ed', cgidir( $mysync ),  'tests_cgidir: Net::Server::HTTP + Docker => /var/tmp/imapsync_cgi/385d...' ) ;
+                                is( '/var/tmp/imapsync_cgi/70348345eb2460c97129e7a3f8ffb8842e90c4e4', cgidir( $mysync ),  'tests_cgidir: Net::Server::HTTP + Docker => /var/tmp/imapsync_cgi/7034...' ) ;
                         } ;
         
                 }
@@ -4268,7 +4268,7 @@ sub cgidir
         {
         
                 $mysync->{ hashfile } = $CGI_HASHFILE ;
-                my $hashsynclocal = hashsynclocal( $mysync ) || die "Can not get hashsynclocal. Exiting\n" ;
+                my $hashsynclocal = hashsynclocal( $mysync, 'uuzvalmsxltblnsvyvdidshqnmmaelym' ) || die "Can not get hashsynclocal. Exiting\n" ;
 
                 if ( ! under_docker_context( $mysync ) and  under_Net_Server_HTTP(  ) )
                 {
@@ -5729,10 +5729,12 @@ sub hashsynclocal
 	if ( ! $hashfile ) {
 		return ;
 	}
-	$hashkey = firstline( $hashfile ) ;
 	if ( ! $hashkey ) {
-		myprint( "No hashkey!\n" ) ;
-		return ;
+	        $hashkey = firstline( $hashfile ) ;
+	        if ( ! $hashkey ) {
+                        myprint( "No hashkey!\n" ) ;
+                        return ;
+	        }
 	}
 	my $hashsynclocal = hashsync( $mysync, $hashkey ) ;
 	return( $hashsynclocal ) ;

[gilleslamiral]

Upon reviewing the code, it seems that the hashsynclocal function generates a different hash due to a different hashkey.

That's the purpose of hashsynclocal(), it generates a different hash on each /X instance, unless they share a common hashkey stored in /var/tmp/imapsync_hash

Therefore, I passed a stable hashkey to the hashsynclocal function in cgidir function, resulting in a stable hash.

I don't want a stable hash across /X unrelated instances.

The goal is to make the log directory path unpredictable for an online service you don't own.

[Xunop]

I don't want a stable hash across /X unrelated instances.

I'm sorry, but if there isn't a stable hash, it implies that the tests_cgidir function will always fail. doesn't it?

[gilleslamiral]

I'm sorry, but if there isn't a stable hash, it implies that the tests_cgidir function will always fail. doesn't it?

Yes. It's a bug.
But this bug is not solved enough with your patch because your patch fixes the failure but crashes the feature.

My turn to fix it, unless you have time with that shit.

[Xunop]

Yes. It's a bug. But this bug is not solved enough with your patch because your patch fixes the failure but crashes the feature.

I apologize for the confusion; it was my mistake.

My turn to fix it, unless you have time with that shit.

I noticed that you updated the imapsync and included a new function called lamiral_host. Now the tests_cgidir function will now be skipped during testing.

Thank you.

[gilleslamiral]

I noticed that you updated the imapsync and included a new function called lamiral_host. Now the tests_cgidir function will now be skipped during testing.

Yes
https://imapsync.lamiral.info/ChangeLog

...
revision 2.276	locked by: gilles;
date: 2023/12/26 17:37:14;  author: gilles;  state: Exp;  lines: +42 -11
Bugfix. Pass --tests everywhere