tests: use gojq - part 1 #14686

bboozzoo · 2024-11-04T15:53:54Z

https://github.com/itchyny/gojq is almost-compatible replacement for jq, with additional features such as native support for YAML input and output.

Attempt to switch the tests to use gojq instead of jq. Where appropriate drop use remarshal, since YAML support is built in.

Related: SNAPDENG-33306

tests/main/aux-info/task.yaml

codecov · 2024-11-04T16:16:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.03%. Comparing base (96ea7b0) to head (2706405).
Report is 80 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #14686      +/-   ##
==========================================
+ Coverage   78.95%   79.03%   +0.07%     
==========================================
  Files        1084     1087       +3     
  Lines      146638   147628     +990     
==========================================
+ Hits       115773   116672     +899     
- Misses      23667    23728      +61     
- Partials     7198     7228      +30

Flag	Coverage Δ
unittests	`79.03% <ø> (+0.07%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

zyga

First pass mostly looks good. I left a few questions about things that seemed like they could be broken out to another polar quest later.

zyga · 2024-11-08T08:08:11Z

tests/lib/prepare-restore.sh

+    # - with --yaml-input, can parse YAML
+    GOBIN=$PROJECT_PATH/tests/bin \
+    CGO_ENABLED=0 \
+        go install github.com/itchyny/gojq/cmd/[email protected]


We might consider just pulling one of the pre-built artefacts instead:

https://github.com/itchyny/gojq/releases/download/v0.12.16/gojq_v0.12.16_linux_amd64.tar.gz

https://github.com/itchyny/gojq/releases/download/v0.12.16/gojq_v0.12.16_linux_arm64.tar.gz

The build could be only done on arch mismatch.

are those built with CGO disabled?

zyga · 2024-11-08T08:46:22Z

tests/main/interfaces-custom-device-app-slot/task.yaml

+    snap ack "$TESTSLIB/assertions/developer1.account"
+    snap ack "$TESTSLIB/assertions/developer1.account-key"


How is this related?

it never worked, but store-state was running without -e, so it failed quietly on https://github.com/canonical/snapd/pull/14686/files/08eab5ab589736cc661f00cffec08f72bcf1f0a7#diff-234265efe637e5730e201767212cce6ce6f4f5fea39bfb026a01913fa281afccL128

I cherry picked along with store-state fixes into #14725

zyga · 2024-11-08T08:48:03Z

tests/main/snap-disconnect/task.yaml


 execute: |
    inspect_connection() {
      CONN="$1"
-      # shellcheck disable=SC2002
-      cat /var/lib/snapd/state.json | jq --arg CONN "$CONN" -r '.data["conns"] | has($CONN)'
+      # shellcheck disable=SC2002,SC2016


What is 2016?

https://www.shellcheck.net/wiki/SC2016 I have no clue why it wasn't picked up before due to -r '... | has ($CONN)'

zyga · 2024-11-08T08:48:49Z

tests/main/store-state/snap/meta/snap.yaml.in

@@ -1,6 +1,7 @@
 name: SNAPNAME
 summary: generic snap
 version: '1.0'
+base: core22


Could we add this in the other pass later?

@bboozzoo ?

I'd prefer to avoid pulling in core because it masked bugs and wrong assumptions in the tests.

zyga · 2024-11-08T08:49:35Z

tests/main/store-state/task.yaml

@@ -40,6 +29,10 @@ execute: |
        snap info core | MATCH "store-url:.*https://snapcraft.io"
    fi

+    # install dependecies before switching to fake store


This seems related to the base change, but I don't really understand the significance. Can we add this in a second pass later?

Not really, fake store does not forward install requests to the real store. The test snap installed here had no base, hence defaulted to 'core'. We installed jq & remarshal both of which pulled in 'core'. Since the install happened before switching over to fake store, it all worked by accident as the dependencies happened to be satisfied. Without jq or remarshal nothing pulls in the dependencies. That's why the snap's base was switched to core22 too, just to avoid pulling in core.

zyga

Two comments:

Let's try to separate changes to bases to another PR so that it is clear they are not a factor here. Use core if you have to have something.

The store endpoint for find is sometimes throttled, perhaps we can use info instead?

olivercalder

Thanks! I think I spotted a few typos, but otherwise looks good.

olivercalder · 2024-11-14T03:11:10Z

tests/nested/manual/core20-install-device-file-install-ubuntu-save-via-hook/task.yaml

+    # slurp our two json files together into one json document, then convert
    # back to yaml and write out to the snap.yaml in the unpacked gadget snap


Suggested change

# slurp our two json files together into one json document, then convert

# back to yaml and write out to the snap.yaml in the unpacked gadget snap

# slurp our two yaml files together into one yaml document, then write

# out to the snap.yaml in the unpacked gadget snap

olivercalder · 2024-11-14T03:11:46Z

tests/nested/manual/core20-install-device-file-install-ubuntu-save-via-hook/task.yaml

    # back to yaml and write out to the snap.yaml in the unpacked gadget snap
-    jq -s '.[0] * .[1]' <(cat snap-yaml-extras.json) <(cat pc-gadget/meta/snap.json) | json2yaml | tee pc-gadget/meta/snap.yaml
+    gojq -s --yaml-input --yaml-output '.[0] * .[1]' <(cat snap-yaml-extras.json) <(cat pc-gadget/meta/snap.json) | \


Suggested change

gojq -s --yaml-input --yaml-output '.[0] * .[1]' <(cat snap-yaml-extras.json) <(cat pc-gadget/meta/snap.json) | \

gojq -s --yaml-input --yaml-output '.[0] * .[1]' <(cat snap-yaml-extras.yaml) <(cat pc-gadget/meta/snap.yaml) | \

olivercalder · 2024-11-14T03:13:00Z

tests/nested/manual/core20-install-device-file-install-via-hook-hack/task.yaml

+    # slurp our two json files together into one json document, then convert
    # back to yaml and write out to the snap.yaml in the unpacked gadget snap


Suggested change

# slurp our two json files together into one json document, then convert

# back to yaml and write out to the snap.yaml in the unpacked gadget snap

# slurp our two yaml files together into one taml document, then write

# out to the snap.yaml in the unpacked gadget snap

maykathm

Nice work

bboozzoo · 2024-11-14T15:31:42Z

I've rebased the branch on top of #14725

olivercalder

Looks good to me, thanks!

Signed-off-by: Maciej Borzecki <[email protected]>

… use gojq Signed-off-by: Maciej Borzecki <[email protected]>

…a-hook: use gojq Signed-off-by: Maciej Borzecki <[email protected]>

…nder tests/bin Since gojq may be running on a different system, make sure to build it statically. Also place the binary under $PROJECT_PATH/tests/bin which is automaticall added to $PATH. Signed-off-by: Maciej Borzecki <[email protected]>

Signed-off-by: Maciej Borzecki <[email protected]>

…k-hack: use gojq Signed-off-by: Maciej Borzecki <[email protected]>

…save-via-hook: use gojq Signed-off-by: Maciej Borzecki <[email protected]>

…k-hack: use gojq Signed-off-by: Maciej Borzecki <[email protected]>

…save-via-hook: use gojq Signed-off-by: Maciej Borzecki <[email protected]>

bboozzoo · 2024-11-18T11:08:03Z

Failures:

google:ubuntu-20.04-64:tests/main/interfaces-microstack-support - unrelated
google-distro-1:debian-11-64:tests/main/registries-cross-config - unrelated
google*:ubuntu--64:tests/main/apparmor-prompting-integration-tests: - unrelated
google-nested:ubuntu-20.04-64:tests/nested/manual/snapd-removes-vulnerable-snap-confine-revs:*
bunch of unrelated UC24 nested:
google-nested:ubuntu-24.04-64:tests/nested/core/core20-kernel-reseal
google-nested:ubuntu-24.04-64:tests/nested/manual/core20-early-config
google-nested:ubuntu-24.04-64:tests/nested/manual/core20-initramfs-time-moves-forward
google-nested:ubuntu-24.04-64:tests/nested/manual/muinstaller-core:install_optional_all
google-nested:ubuntu-24.04-64:tests/nested/manual/muinstaller-core:install_optional_snap
google-nested:ubuntu-24.04-64:tests/nested/manual/muinstaller-core:plain
google-nested:ubuntu-24.04-64:tests/nested/manual/muinstaller-real:partial
google-nested:ubuntu-24.04-64:tests/nested/manual/muinstaller-real:plain
google-nested:ubuntu-24.04-64:tests/nested/manual/remodel-min-size
google-nested:ubuntu-24.04-64:tests/nested/manual/core20-4k-sector-size:logical
google-nested:ubuntu-24.04-64:tests/nested/manual/devmode-snap-seeded-dangerous
google-nested:ubuntu-24.04-64:tests/nested/manual/hybrid-remodel

zyga

Looks good to me.

Thank you for pushing this forward.

This reverts commit ede0457.

* master: (44 commits) wrappers: do not reload activation units (canonical#14724) gadget/install: support for no{exec,dev,suid} mount flags interfaces/builtin/mount_control: add support for nfs mounts (canonical#14694) tests: use gojq - part 1 (canonical#14686) interfaces/desktop-legacy: allow DBus access to com.canonical.dbusmenu interfaces/builtin/fwupd.go: allow access to nvmem for thunderbolt plugin tests: removing uc16 executions (canonical#14575) tests: Added arm github runner to build snapd (canonical#14504) tests: no need to run spread when there are not tests matching the filter (canonical#14728) tests/lib/tools/store-state: exit on errors, update relevant tests (canonical#14725) tests: udpate the github workflow to run tests suggested by spread-filter tool (canonical#14519) testtime: add mockable timers for use in tests (canonical#14672) interface/screen_inhibit_control: Improve screen inhibit control for use on core (canonical#14134) tests: use images with 20G disk in openstack (canonical#14720) i/builtin: allow @ in custom-device filepaths (canonical#14651) tests: refactor test-snapd-desktop-layout-with-content tests: fix broken app definition tests: capitalize sentences in comments tests: wrap very long shell line tests: fix raciness in async startup and sync install ...

bboozzoo added ⛔ Blocked Run nested The PR also runs tests inluded in nested suite labels Nov 4, 2024

bboozzoo requested review from olivercalder, zyga and ZeyadYasser November 4, 2024 15:54

github-actions bot added the Run Nested -auto- Label automatically added in case nested tests need to be executed label Nov 4, 2024

bboozzoo requested a review from sergiocazzolato November 4, 2024 15:54

valentindavid reviewed Nov 4, 2024

View reviewed changes

tests/main/aux-info/task.yaml Show resolved Hide resolved

bboozzoo mentioned this pull request Nov 6, 2024

tests/main/auto-refresh-backoff: fix sorting of changes from the state #14691

Merged

bboozzoo force-pushed the bboozzoo/jq-to-gojq branch from 662aad6 to 08eab5a Compare November 7, 2024 10:57

zyga reviewed Nov 8, 2024

View reviewed changes

bboozzoo force-pushed the bboozzoo/jq-to-gojq branch from 08eab5a to 2047373 Compare November 12, 2024 12:09

bboozzoo marked this pull request as ready for review November 12, 2024 12:14

bboozzoo removed the ⛔ Blocked label Nov 12, 2024

bboozzoo force-pushed the bboozzoo/jq-to-gojq branch 2 times, most recently from e781775 to fbe5e86 Compare November 13, 2024 11:13

bboozzoo requested review from zyga and maykathm November 13, 2024 11:13

zyga reviewed Nov 13, 2024

View reviewed changes

bboozzoo changed the title ~~tests: use gojq [WIP]~~ tests: use gojq - part 1 Nov 13, 2024

olivercalder requested changes Nov 14, 2024

View reviewed changes

bboozzoo force-pushed the bboozzoo/jq-to-gojq branch from fbe5e86 to d970c4b Compare November 14, 2024 10:35

bboozzoo requested a review from olivercalder November 14, 2024 10:35

bboozzoo mentioned this pull request Nov 14, 2024

tests/lib/tools/store-state: exit on errors, update relevant tests #14725

Merged

maykathm approved these changes Nov 14, 2024

View reviewed changes

bboozzoo force-pushed the bboozzoo/jq-to-gojq branch from d970c4b to fd699d5 Compare November 14, 2024 15:31

olivercalder approved these changes Nov 14, 2024

View reviewed changes

bboozzoo force-pushed the bboozzoo/jq-to-gojq branch from fd699d5 to 16c94fa Compare November 15, 2024 10:21

bboozzoo added 21 commits November 18, 2024 09:32

tests/core/auto-refresh-backoff-after-reboot: use gojq

e21c768

Signed-off-by: Maciej Borzecki <[email protected]>

tests/core/uc20-recovery: use gojq

d1f9f70

Signed-off-by: Maciej Borzecki <[email protected]>

tests/core/snapd-refresh-vs-services: use gojq

f883d16

Signed-off-by: Maciej Borzecki <[email protected]>

tests/main/apparmor-prompting-flag-restart: use gojq

db2d443

Signed-off-by: Maciej Borzecki <[email protected]>

tests/main/quota-groups-systemd-accounting: use gojq

bcea77a

Signed-off-by: Maciej Borzecki <[email protected]>

tests/main/snapd-reexec: use gojq

a8c3376

Signed-off-by: Maciej Borzecki <[email protected]>

tests/main/snap-get: use gojq

2ef3c84

Signed-off-by: Maciej Borzecki <[email protected]>

tests/main/snap-disconnect: use gojq

68692b4

Signed-off-by: Maciej Borzecki <[email protected]>

tests/main/auto-refresh-gating-from-snap: use gojq

0f3d762

Signed-off-by: Maciej Borzecki <[email protected]>

tests/main/cloud-init: use gojq

617ea77

Signed-off-by: Maciej Borzecki <[email protected]>

tests/core/snapd-maintenance-msg: use gojq

d928e26

Signed-off-by: Maciej Borzecki <[email protected]>

tests/nested/manual/core20-validation-sets: drop jq

877b75a

Signed-off-by: Maciej Borzecki <[email protected]>

tests/nested/manual/core20-install-device-file-install-via-hook-hack:…

48c238d

… use gojq Signed-off-by: Maciej Borzecki <[email protected]>

tests/nested/manual/core20-install-device-file-install-ubuntu-save-vi…

34852bc

…a-hook: use gojq Signed-off-by: Maciej Borzecki <[email protected]>

tests/main/store-state: reach out to real store before proceeding

d19b50a

Signed-off-by: Maciej Borzecki <[email protected]>

fixup! tests/core/auto-refresh-backoff-after-reboot: use gojq

a2640d0

Signed-off-by: Maciej Borzecki <[email protected]>

fixup! tests/nested/manual/core20-install-device-file-install-via-hoo…

5ac921a

…k-hack: use gojq Signed-off-by: Maciej Borzecki <[email protected]>

fixup! tests/nested/manual/core20-install-device-file-install-ubuntu-…

e153fdc

…save-via-hook: use gojq Signed-off-by: Maciej Borzecki <[email protected]>

fixup! tests/nested/manual/core20-install-device-file-install-via-hoo…

c28af61

…k-hack: use gojq Signed-off-by: Maciej Borzecki <[email protected]>

fixup! tests/nested/manual/core20-install-device-file-install-ubuntu-…

2706405

…save-via-hook: use gojq Signed-off-by: Maciej Borzecki <[email protected]>

bboozzoo force-pushed the bboozzoo/jq-to-gojq branch from 6ff0ed3 to 2706405 Compare November 18, 2024 08:32

ernestl requested a review from valentindavid November 18, 2024 12:58

zyga approved these changes Nov 18, 2024

View reviewed changes

bboozzoo added this to the 2.67 milestone Nov 18, 2024

ernestl merged commit ede0457 into canonical:master Nov 18, 2024
47 of 56 checks passed

sergiocazzolato added a commit to sergiocazzolato/snapd that referenced this pull request Nov 18, 2024

Revert "tests: use gojq - part 1 (canonical#14686)"

8c36916

This reverts commit ede0457.

bboozzoo deleted the bboozzoo/jq-to-gojq branch November 19, 2024 12:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: use gojq - part 1 #14686

tests: use gojq - part 1 #14686

bboozzoo commented Nov 4, 2024

codecov bot commented Nov 4, 2024 •

edited

Loading

zyga left a comment

zyga Nov 8, 2024

bboozzoo Nov 12, 2024

zyga Nov 8, 2024

bboozzoo Nov 12, 2024

bboozzoo Nov 14, 2024

zyga Nov 8, 2024

bboozzoo Nov 12, 2024

zyga Nov 8, 2024

zyga Nov 13, 2024

bboozzoo Nov 13, 2024

zyga Nov 8, 2024

bboozzoo Nov 12, 2024

zyga left a comment

olivercalder left a comment

olivercalder Nov 14, 2024

olivercalder Nov 14, 2024

olivercalder Nov 14, 2024

maykathm left a comment

bboozzoo commented Nov 14, 2024

olivercalder left a comment

bboozzoo commented Nov 18, 2024

zyga left a comment

		snap ack "$TESTSLIB/assertions/developer1.account"
		snap ack "$TESTSLIB/assertions/developer1.account-key"

		# slurp our two json files together into one json document, then convert
		# back to yaml and write out to the snap.yaml in the unpacked gadget snap

	gojq -s --yaml-input --yaml-output '.[0] * .[1]' <(cat snap-yaml-extras.json) <(cat pc-gadget/meta/snap.json) \| \
	gojq -s --yaml-input --yaml-output '.[0] * .[1]' <(cat snap-yaml-extras.yaml) <(cat pc-gadget/meta/snap.yaml) \| \

tests: use gojq - part 1 #14686

tests: use gojq - part 1 #14686

Conversation

bboozzoo commented Nov 4, 2024

codecov bot commented Nov 4, 2024 • edited Loading

Codecov Report

zyga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zyga left a comment

Choose a reason for hiding this comment

olivercalder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maykathm left a comment

Choose a reason for hiding this comment

bboozzoo commented Nov 14, 2024

olivercalder left a comment

Choose a reason for hiding this comment

bboozzoo commented Nov 18, 2024

zyga left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 4, 2024 •

edited

Loading