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

Fix subType streaming validator errors #272

Merged
merged 15 commits into from
Mar 17, 2022
Merged

Conversation

grafnu
Copy link
Collaborator

@grafnu grafnu commented Mar 16, 2022

No description provided.

bin/validator Outdated Show resolved Hide resolved
@grafnu grafnu requested a review from noursaidi March 17, 2022 14:36
@grafnu grafnu merged commit 6686d24 into faucetsdn:master Mar 17, 2022
@grafnu grafnu deleted the vfixing branch March 17, 2022 17:23
- name: validation logs
fgrep 'RESULT ' /tmp/test_log.txt | sed -e 's/.*sequencer RESULT/RESULT/' > /tmp/sequencer.out
diff -u /tmp/sequencer.out etc/sequencer.out && echo No output diff detected.
- name: device ouput logs
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: output

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll fold this into another PR


PUBBER_OUT=pubber.out

pids=`ps ax | fgrep pubber | fgrep java | awk '{print $1}'`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels heavy handed? Is this to strengthen repeated runs of this test, or to stop interference with other running tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes -- but I'd defer any of those changes to something separate from the basic functionality. I agree with your comments, but I wouldn't want to get into it as part of this PR!

cp udmi_site_model/devices/AHU-1/rsa_private.pkcs8 \
udmi_site_model/gcp_reflect_key.pkcs8

bin/validator $site_path $project_id &
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we trap on exit of validator so we know if it exits while running pubber?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe-- the code as it stands checks the output of what validator does, so it doesn't matter if it exits prematurely or not... Either the generated output is correct or not.


echo Waiting for system to run for a bit...
sleep 30

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this becomes more cumbersome, or we get unpredictable test results, I suggest using the shell's jobs command to focus our kill usage

For example:

jobs -x kill %pubber
jobs -x kill %validator

Although I'm not sure whether the shell would see either java or pubber/validator


outfiles=`find out/devices -name \*.out` || true
echo Found .out files $outfiles, copying to /tmp/validator.out
more $outfiles > /tmp/validator.out || true
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be cat?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

more includes the filename in the output, so it's convenient for logging since you get more context, rather than just a bunch of stuff combined together

echo Waiting for system to run for a bit...
sleep 30

pids=`ps ax | fgrep pubber | fgrep java | awk '{print $1}'`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Or at least pids=`pgrep pubber` ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants