Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proviers/base: add requires for bluetooth4/beacon_eddystone_url_* (New) #1646

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions providers/base/units/bluetooth/jobs.pxu
Original file line number Diff line number Diff line change
Expand Up @@ -276,17 +276,23 @@ requires:
unit: template
template-resource: device
template-filter: device.category == 'BLUETOOTH'
template-engine: jinja2
template-unit: job
id: bluetooth4/beacon_eddystone_url_{interface}
id: bluetooth4/beacon_eddystone_url_{{ interface }}
template-id: bluetooth4/beacon_eddystone_url_interface
_summary: Test system can get beacon EddyStone URL advertisements on the {interface} adapter
_summary: Test system can get beacon EddyStone URL advertisements on the {{ interface }} adapter
command:
checkbox-support-eddystone_scanner -D {interface}
checkbox-support-eddystone_scanner -D {{ interface }}
plugin: shell
user: root
flags: also-after-suspend
category_id: com.canonical.plainbox::bluetooth
estimated_duration: 10
requires:
package.name == 'bluez' or snap.name == 'bluez'
Comment on lines +291 to +292
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use this statement, and the bluez snap/package is missing, the test will be skipped, which might be even harder to spot if this test was supposed to be executed to begin with.

Is this what you want to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pieqq,

  • I see this statement is used for other tests in this jobs file, that is why I had the idea to do the same for this test
  • You are right about the fact that we have to check all the skipped tests to make sure that are skipped for good reason, but without this statement, I also had a hard time to figure out that this test failed because bluez was missing, i have the feeling that it is a global discussion we might have because expressing dependency of a test on a binary availability is very common in our existing tests, so your concern is somehow global and not applicable to only this PR,

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also having a fail-on-resource flag in this job will get what you want

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @zongminl, done in 9694748

{%- if __on_ubuntucore__ %}
connections.slot == 'bluez:service' and connections.plug == '{{ __system_env__["SNAP_NAME"] }}:bluez'
{% endif -%}

unit: template
template-resource: bluez-internal-rfcomm-tests
Expand Down
Loading