[PATCHv2] git-p4: ticket expiry test: use a fake p4 to avoid use of 'sleep'

[PATCHv2] git-p4: ticket expiry test: use a fake p4 to avoid use of 'sleep'

[Luke Diamand]

From: Luke Diamand <ldiamand@roku.com>

This tests for p4 ticket expiry by creating a ticket, and then waiting
long enough for the ticket to nearly expire. However, this is
unreliable.

Instead, create a 'fake' p4 which returns expiry times under the
control of the test script, and forwards other commands to the
real p4 executable.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 t/t9833-errors.sh | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/t/t9833-errors.sh b/t/t9833-errors.sh
index 277d347012..9d7cb5b35b 100755
--- a/t/t9833-errors.sh
+++ b/t/t9833-errors.sh
@@ -45,30 +45,32 @@ test_expect_success 'ticket logged out' '
 	)
 '
 
-test_expect_success 'create group with short ticket expiry' '
-	P4TICKETS="$cli/tickets" &&
-	echo "newpassword" | p4 login &&
-	p4_add_user short_expiry_user &&
-	p4 -u short_expiry_user passwd -P password &&
-	p4 group -i <<-EOF &&
-	Group: testgroup
-	Timeout: 3
-	Users: short_expiry_user
-	EOF
-
-	p4 users | grep short_expiry_user
-'
+# create a fake version of "p4" which returns a TicketExpiration based
+# on $EXPIRY, for testing login expiration
+create_fake_p4() {
+	(
+		cd "$git" && mkdir expire-p4 &&
+		cat >>expire-p4/p4 <<-EOF &&
+		#!/usr/bin/python
+		import marshal, os, subprocess, sys
+		if "login" in sys.argv:
+		    marshal.dump({"foo" : "bar", "code" : "stat", "TicketExpiration" : os.environ["EXPIRY"]}, sys.stdout)
+		else:
+		    subprocess.check_call([os.environ["P4"]] + sys.argv[1:])
+		EOF
+		chmod 0755 expire-p4/p4
+	)
+}
 
 test_expect_success 'git operation with expired ticket' '
-	P4TICKETS="$cli/tickets" &&
-	P4USER=short_expiry_user &&
-	echo "password" | p4 login &&
+	create_fake_p4 &&
+	echo "newpassword" | p4 login &&
 	(
 		cd "$git" &&
-		git p4 sync &&
-		sleep 5 &&
-		test_must_fail git p4 sync 2>errmsg &&
-		grep "failure accessing depot" errmsg
+		P4=$(command -v p4) && export P4 &&
+		EXPIRY=3600 PATH=$PWD/expire-p4:$PATH git p4 sync &&
+		EXPIRY=1 PATH=$PWD/expire-p4:$PATH test_must_fail git p4 sync -v 2>errmsg &&
+		grep "failure accessing depot.*expires in 1 second" errmsg
 	)
 '
 
-- 
2.20.1.612.g17ebf93fb6.dirty

Re: [PATCHv2] git-p4: ticket expiry test: use a fake p4 to avoid use of 'sleep'

[SZEDER Gábor]

On Fri, Feb 08, 2019 at 07:02:31PM +0000, Luke Diamand wrote:
> +# create a fake version of "p4" which returns a TicketExpiration based
> +# on $EXPIRY, for testing login expiration
> +create_fake_p4() {
> +	(
> +		cd "$git" && mkdir expire-p4 &&
> +		cat >>expire-p4/p4 <<-EOF &&
> +		#!/usr/bin/python

I think this should be $PYTHON_PATH.

> +		import marshal, os, subprocess, sys
> +		if "login" in sys.argv:
> +		    marshal.dump({"foo" : "bar", "code" : "stat", "TicketExpiration" : os.environ["EXPIRY"]}, sys.stdout)
> +		else:
> +		    subprocess.check_call([os.environ["P4"]] + sys.argv[1:])
> +		EOF
> +		chmod 0755 expire-p4/p4
> +	)
> +}
>  
>  test_expect_success 'git operation with expired ticket' '
> -	P4TICKETS="$cli/tickets" &&
> -	P4USER=short_expiry_user &&
> -	echo "password" | p4 login &&
> +	create_fake_p4 &&
> +	echo "newpassword" | p4 login &&
>  	(
>  		cd "$git" &&
> -		git p4 sync &&
> -		sleep 5 &&
> -		test_must_fail git p4 sync 2>errmsg &&
> -		grep "failure accessing depot" errmsg
> +		P4=$(command -v p4) && export P4 &&
> +		EXPIRY=3600 PATH=$PWD/expire-p4:$PATH git p4 sync &&
> +		EXPIRY=1 PATH=$PWD/expire-p4:$PATH test_must_fail git p4 sync -v 2>errmsg &&
> +		grep "failure accessing depot.*expires in 1 second" errmsg
>  	)
>  '
>  
> -- 
> 2.20.1.612.g17ebf93fb6.dirty
> 

Re: [PATCHv2] git-p4: ticket expiry test: use a fake p4 to avoid use of 'sleep'

[Junio C Hamano]

SZEDER Gábor <szeder.dev@gmail.com> writes:

> On Fri, Feb 08, 2019 at 07:02:31PM +0000, Luke Diamand wrote:
>> +# create a fake version of "p4" which returns a TicketExpiration based
>> +# on $EXPIRY, for testing login expiration
>> +create_fake_p4() {
>> +	(
>> +		cd "$git" && mkdir expire-p4 &&
>> +		cat >>expire-p4/p4 <<-EOF &&
>> +		#!/usr/bin/python
>
> I think this should be $PYTHON_PATH.

OK.

Luke, I think our mails / work crossed and the tip of 'master'
already has a removal.  Please make this into a separate patch that
adds (or, resurrects with an improved version) the test for the next
cycle.

Thanks.

Re: [PATCHv2] git-p4: ticket expiry test: use a fake p4 to avoid use of 'sleep'

[Luke Diamand]

On Sat, 9 Feb 2019 at 18:06, Junio C Hamano <gitster@pobox.com> wrote:
>
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
> > On Fri, Feb 08, 2019 at 07:02:31PM +0000, Luke Diamand wrote:
> >> +# create a fake version of "p4" which returns a TicketExpiration based
> >> +# on $EXPIRY, for testing login expiration
> >> +create_fake_p4() {
> >> +    (
> >> +            cd "$git" && mkdir expire-p4 &&
> >> +            cat >>expire-p4/p4 <<-EOF &&
> >> +            #!/usr/bin/python
> >
> > I think this should be $PYTHON_PATH.
>
> OK.
>
> Luke, I think our mails / work crossed and the tip of 'master'
> already has a removal.  Please make this into a separate patch that
> adds (or, resurrects with an improved version) the test for the next
> cycle.

OK, thanks!