Merge ~utkarsh/ubuntu/+source/at:lp1677748-add-dep8-tests into ubuntu/+source/at:ubuntu/devel

Proposed by Utkarsh Gupta
Status: Merged
Approved by: Bryce Harrington
Approved revision: 2d3edc970a8cd5d44eef23a87d95539c7bedd9cd
Merged at revision: 2d3edc970a8cd5d44eef23a87d95539c7bedd9cd
Proposed branch: ~utkarsh/ubuntu/+source/at:lp1677748-add-dep8-tests
Merge into: ubuntu/+source/at:ubuntu/devel
Diff against target: 91 lines (+71/-0)
3 files modified
debian/changelog (+7/-0)
debian/tests/basic-usage (+61/-0)
debian/tests/control (+3/-0)
Reviewer Review Type Date Requested Status
Bryce Harrington (community) Approve
Canonical Server Pending
Canonical Server packageset reviewers Pending
Review via email: mp+399718@code.launchpad.net

Description of the change

Hello,

This MP adds a DEP8 test for the at package, using and for at and atq functionalities.

This is a trivial but good test to catch regressions, if there's any. I also intend to send this to Debian hopefully after this is merged here and uploaded.

PPA at https://launchpad.net/~utkarsh/+archive/ubuntu/experimental-dump.

Autopkgtest is clean and happy as well:
```
autopkgtest [02:20:37]: test basic-usage: -----------------------]
autopkgtest [02:20:37]: test basic-usage: - - - - - - - - - - results - - - - - - - - - -
basic-usage PASS
autopkgtest [02:20:38]: @@@@@@@@@@@@@@@@@@@@ summary
```

Requesting y'all to please review and sponsor the upload. TIA!

To post a comment you must log in.
Revision history for this message
Bryce Harrington (bryce) wrote :

* Changelog:
  - [-] old content and logical tag match as expected
  - [√] changelog entry correct version and targeted codename
  - [√] changelog entries correct
  - [-] update-maintainer has been run

* Actual changes:
  - [-] no upstream changes to consider
  - [-] no further upstream version to consider
  - [√] debian changes look safe

* Old Delta:
  - [-] dropped changes are ok to be dropped
  - [-] nothing else to drop
  - [√] changes forwarded upstream/debian (if appropriate)
    + (Yes, mp indicates will be forwarded subsequently)

* New Delta:
  - [√] no new patches added
  - [-] patches match what was proposed upstream
  - [-] patches correctly included in debian/patches/series
  - [-] patches have correct DEP3 metadata

* Build/Test:
  - [√] build is ok
  - [√] verified PPA package installs/uninstalls
  - [√] autopkgtest against the PPA package passes
  - [√] sanity checks test fine

Some review comments inline below. Most are suggestions and you can take or not as you wish, the only actual coding error is the '-eq' -> '=' one.

DEP8 passes for autopkgtest when run with an ephemeral container, however I did find a situation where it will fail to pass (see below).

review: Needs Fixing
Revision history for this message
Bryce Harrington (bryce) wrote :

Hmm, launchpad didn't save my code comments for some reason. That's annoying...

Revision history for this message
Bryce Harrington (bryce) wrote :

I've redone the comments, let's see if they save this time.

Revision history for this message
Utkarsh Gupta (utkarsh) wrote :

Hi Bryce,

Thanks for the great review. I've adjusted the MP according to the suggestions proposed. The only bit remaining is "skipping error checking in favor of set -ex". I'd personally like explicit checks (as do you) because it really helps in debugging and reading through as to what it was supposed to do v/s what's happening.

Whilst this is a trivial test and it's easier to figure out this but in more complex cases, I'd personally use them to be explicitly explicit.

I'm OK in removing those as well if that's still preferable and if you'd like me to do so. Please let me know! \o/

Revision history for this message
Bryce Harrington (bryce) wrote :

LGTM, +1

And yeah like mentioned while there are common conventions there is lots of room for personal style preferences.

review: Approve
Revision history for this message
Bryce Harrington (bryce) wrote :

Upload sponsored:

$ dput ubuntu ../at_3.1.23-1.1ubuntu3_source.changes
D: Setting host argument.
Checking signature on .changes
gpg: ../at_3.1.23-1.1ubuntu3_source.changes: Valid signature from E603B2578FB8F0FB
Checking signature on .dsc
gpg: ../at_3.1.23-1.1ubuntu3.dsc: Valid signature from E603B2578FB8F0FB
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading at_3.1.23-1.1ubuntu3.dsc: done.
  Uploading at_3.1.23-1.1ubuntu3.debian.tar.xz: done.
  Uploading at_3.1.23-1.1ubuntu3_source.buildinfo: done.
  Uploading at_3.1.23-1.1ubuntu3_source.changes: done.
Successfully uploaded packages.
$ git ubuntu tag --upload
$ git push pkg upload/3.1.23-1.1ubuntu3
Enumerating objects: 14, done.
Counting objects: 100% (14/14), done.
Delta compression using up to 12 threads
Compressing objects: 100% (11/11), done.
Writing objects: 100% (11/11), 1.93 KiB | 197.00 KiB/s, done.
Total 11 (delta 5), reused 0 (delta 0), pack-reused 0
To ssh://git.launchpad.net/ubuntu/+source/at
 * [new tag] upload/3.1.23-1.1ubuntu3 -> upload/3.1.23-1.1ubuntu3

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 09ff2a6..c5afc6c 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,10 @@
6+at (3.1.23-1.1ubuntu3) hirsute; urgency=medium
7+
8+ * d/t/{control, basic-usage}: Add DEP8 test
9+ for testing basic usage of at and atq commands. (LP: #1677748)
10+
11+ -- Utkarsh Gupta <utkarsh.gupta@canonical.com> Tue, 16 Mar 2021 18:27:13 +0530
12+
13 at (3.1.23-1.1ubuntu2) hirsute; urgency=medium
14
15 * d/patches: add 03-do-not-drop-seconds.patch (cherry-pick).
16diff --git a/debian/tests/basic-usage b/debian/tests/basic-usage
17new file mode 100755
18index 0000000..7f3eb4c
19--- /dev/null
20+++ b/debian/tests/basic-usage
21@@ -0,0 +1,61 @@
22+#!/bin/bash
23+
24+#################################################
25+## ##
26+## Basic usage of `at` to create a file and ##
27+## also take in account the queue, using atq. ##
28+## ##
29+## Written by: ##
30+## Utkarsh Gupta <utkarsh.gupta@canonical.com> ##
31+## License: GPL-2+ ##
32+## ##
33+#################################################
34+
35+set -ex
36+
37+TMPFILE=at.$RANDOM
38+WORKDIR=$(mktemp -d)
39+trap "rm -rf $WORKDIR" 0 INT QUIT ABRT PIPE TERM
40+
41+JOBS_BEFORE=$(atq | wc -l)
42+
43+# use at command to schedule a job.
44+echo "echo `date` > ${WORKDIR}/${TMPFILE}" | at now + 1 minute
45+
46+sleep 2
47+
48+# check if the file is created at the wrong
49+# time, that is, 58 seconds earlier.
50+if test -f "$TMPFILE"
51+then
52+ echo "$TMPFILE already exits but it really shouldn't."
53+ exit 1
54+else
55+ echo "OK, $TMPFILE doesn't exist yet; expected.."
56+fi
57+
58+# check atq to see the queued jobs.
59+# there should be a one new job.
60+JOBS_AFTER=$(atq | wc -l)
61+
62+if [[ $((JOBS_AFTER - JOBS_BEFORE)) -eq 1 ]]
63+then
64+ echo "OK, 1 new queued job exists.."
65+else
66+ echo "No new queued job found."
67+ exit 1
68+fi
69+
70+sleep 58
71+
72+# grep the content of the file to verify that
73+# everything is in order.
74+if grep -Fq "UTC" $WORKDIR/$TMPFILE
75+then
76+ echo "OK, $TMPFILE exists and everything looks in order.."
77+else
78+ echo "Either $TMPFILE doesn't exist or the content differs."
79+ exit 1
80+fi
81+
82+echo "OK; PASS."
83diff --git a/debian/tests/control b/debian/tests/control
84new file mode 100644
85index 0000000..ccc00c6
86--- /dev/null
87+++ b/debian/tests/control
88@@ -0,0 +1,3 @@
89+Tests: basic-usage
90+Depends: @
91+Restrictions: allow-stderr

Subscribers

People subscribed via source and target branches