Create autopkgtest

There are issues with the proposed diff.

1. the d/changelog entry is not descriptive of what the added test does. Please add something like (you'll know better):

 * d/tests: add basic connection test (LP: #....)

2. the debdiff adds d/t/a.sh which is unused.

3. d/t/connect: better not specify shell options in the shebang, as that get lost if the script is invoked with `sh /path/to/script`. Use `set`.

4. d/t/connect: isNaturalNumber() returns 0 on the empty string, but we don't want that.

5. d/t/connect: this logic:

if isNaturalNumber "$port"; then
    exit 1
fi

seems backwards to me: don't we want $port to be a natural number? FWIW I think it's enough to check that it's a nonempty string ([ -n "$port" ]).

6. xrdp.service has WantedBy=multi-user.target. Why do we need to manually `sudo systemctl start xrdp`?

7. Can we make the `timeout 4s` longer? In general, can you add a comment on what we are testing more precisely?

Hi Paride, thanks for the review.

I've addressed your comments, except for number 6. For some reason this
failed when I tried it in the past. But if necessary I can try again and report
with the resulting error, as this I don't remember.

1/ d/changelog should refer to oracular now, not noble

2.
+# XXX: Is there a better way to get the port number other than by parsing the
+# configuration file?
+port=$(sed -n '/^port=[0123456789]/{s/port=//p;q;}' /etc/xrdp/xrdp.ini)

Does the port change randomly, or is it always 3389 on a fresh install?

3.
+if test -n "$port"; then
+ exit 1
+fi

I think you meant "test -z" instead.

+timeout 29s xvfb-run -l xfreerdp /v:localhost:"$port" /p: /u: /d:
+if test $? != 124; then
+ 2>&1 printf "%s\n" "Xfreerdp exited before the timeout, it has likely " \
+ "failed to connect. The test has therefore failed."
+ exit 2

Ok, this is a new type of test :). You are actually counting on the timeout command killing the binary for the test to succeed.

I'm not sure paride understood that before when he asked to increase this timeout. I think your original 4s was ok, because a failure to start should be quick.

I'm wondering if there is something else we could do here, but I don't think there are text-mode rdp clients out there ;) Do you know of something else that could exercise this rdp service at least a little bit more? Perhaps some auth negotiation? I'll leave that up to you to think about while you address the other points above, and it's not a blocker if there isn't anything else to add here that would be simple enough, yet give us a better idea of how well the package is working.

1. Fixed.

2. I'm hard coding it, we need to trust upstream to never change that value.

3. No longer applies.

> Do you know of
> something else that could exercise this rdp service at least a little
> bit more?

In this new patch I take a screen shot of the X framebuffer to check that it
really contains the XRDP log in page.

Test log: https://autopkgtest.ubuntu.com/results/autopkgtest-oracular-nteodosio-rebuilds/oracular/amd64/f/freerdp2/20240527_115526_4de85@/log.gz

I do like the addition of the compare test; it's a bit "magic" but the fact it's comparing against an image from the actual installation (/usr/share/xrdp/xrdp_logo.bmp) does demonstrate the operation of the package very nicely.

I did try and run the tests locally under autopkgtest but unfortunately they fail because the test is using "sudo" without declaring "needs-sudo" in the restrictions (without this, sudo isn't installed/configured for the test suite). Having patched that, the tests failed to run in a chroot due to the presence of needs-sudo (unsurprisingly), so I attempted with a full VM, but that failed with a badpkg error in imagemagick. So, this looks promising, but may need a bit more work to be certain it'll pass in production.

For me, both the Launchpad runner and a local test with

  autopkgtest -B . -- null

succeed if I delete the

  sudo systemctl start xrdp

line. I must have misremembered when I wrote #3. Do you observe failure in any
of your environments if you delete that line?