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?

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to freerdp2 in Ubuntu.
https://bugs.launchpad.net/bugs/2060976

Title:
  Create autopkgtest

Status in freerdp2 package in Ubuntu:
  New

Bug description:
  Ran successfully with autopkgtest -B . -- schroot noble-amd64.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/freerdp2/+bug/2060976/+subscriptions


-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to     : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

Reply via email to