-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add endpoint_config_connection_error test #455
Conversation
validator/src/main/java/com/google/daq/mqtt/sequencer/sequences/ConfigSequences.java
Outdated
Show resolved
Hide resolved
validator/src/main/java/com/google/daq/mqtt/sequencer/sequences/ConfigSequences.java
Outdated
Show resolved
Hide resolved
deviceConfig.blobset.blobs.put("_iot_endpoint_config", cfg); | ||
|
||
updateConfig(); | ||
untilTrue("pubber has attempted endpoint reconfig", () -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not always pubber -- this might be running against a manufacturer device
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right PTAL
validator/src/main/java/com/google/daq/mqtt/sequencer/sequences/ConfigSequences.java
Outdated
Show resolved
Hide resolved
BlobBlobsetConfig cfg = new BlobBlobsetConfig(); | ||
cfg.phase = BlobPhase.FINAL; | ||
// { protocol=mqtt, client_id=test_project/device; hostname=localhost } | ||
cfg.base64 = "ewogICJwcm90b2NvbCI6ICJtcXR0IiwKICAiY2xpZW50X2lkIjogInRlc3RfcHJvamVjdC9kZXZp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extract to named constant for base64 for readability?
validator/src/main/java/com/google/daq/mqtt/sequencer/sequences/ConfigSequences.java
Outdated
Show resolved
Hide resolved
Yes -- this is a very very common case for many tests, which is why I
pulled the functionality into untilTrue -- it makes the code a lot cleaner
overall because all the try/catch blocks are essentially noise to the
underlying test-specific logic. Also, this way it's easier to have the
configured loglevel handle the exception appropriately -- e.g. mostly
silently ignore except when in verbose mode, and then show what's going
on...
…On Fri, Sep 16, 2022 at 10:33 AM John R ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
validator/src/main/java/com/google/daq/mqtt/sequencer/sequences/ConfigSequences.java
<#455 (comment)>:
> + @description("Push endpoint config message to device that results in a connection error.")
+ public void endpoint_config_connection_error() {
+ // blob _iot_endpoint_config = ...
+ BlobBlobsetConfig cfg = new BlobBlobsetConfig();
+ cfg.phase = BlobPhase.FINAL;
+ // { protocol=mqtt, client_id=test_project/device; hostname=localhost }
+ cfg.base64 = "ewogICJwcm90b2NvbCI6ICJtcXR0IiwKICAiY2xpZW50X2lkIjogInRlc3RfcHJvamVjdC9kZXZp"
+ + "Y2UiLAogICJob3N0bmFtZSI6ICJsb2NhbGhvc3QiCn0K";
+ cfg.content_type = "application/json";
+ deviceConfig.blobset = new BlobsetConfig();
+ deviceConfig.blobset.blobs = new HashMap<String, BlobBlobsetConfig>();
+ deviceConfig.blobset.blobs.put("_iot_endpoint_config", cfg);
+
+ updateConfig();
+ untilTrue("pubber has attempted endpoint reconfig", () -> {
+ try {
Even though we will almost certainly NPE > 1 ?
—
Reply to this email directly, view it on GitHub
<#455 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIEPD4JZ53SOATHWBIY553V6SVMZANCNFSM6AAAAAAQOOAOCY>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
validator/src/main/java/com/google/daq/mqtt/sequencer/sequences/BlobsetSequences.java
Outdated
Show resolved
Hide resolved
import org.junit.Test; | ||
import udmi.schema.BlobBlobsetConfig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra stuff not necessary anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which thing is extra?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the changes in the file... aren't they leftover from when you had the test itself in there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh haha. Yes of course. One moment
validator/src/main/java/com/google/daq/mqtt/sequencer/sequences/BlobsetSequences.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code here looks fine, but I'd expect there to be some accompanying generated.md changes? It should automatically pick up that there's a new test... so if there isn't I'd like to know so I can track that down!
deviceConfig.blobset.blobs.put(String.valueOf(SystemBlobsets.IOT_ENDPOINT_CONFIG), config); | ||
|
||
untilTrue("device tried endpoint config which resulted in connection error", () -> { | ||
Entry stateStatus = deviceState.blobset.blobs.get(SystemBlobsets.IOT_ENDPOINT_CONFIG).status; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this works it's fine, but I'd be wondering why .value() isn't necessary here when it is elsewhere... basically if this doesn't work for some reason just something to consider!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk, I let intellij ask for the other value()
validator/src/main/java/com/google/daq/mqtt/sequencer/sequences/BlobsetSequences.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok PTAL
No description provided.