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

Add key publisher test #477

Merged
merged 2 commits into from
Nov 7, 2022
Merged

Add key publisher test #477

merged 2 commits into from
Nov 7, 2022

Conversation

jennuine
Copy link
Contributor

Part of gazebosim/gz-sim#1575

Summary

Added KeyPublisher_TEST to increase code coverage

Checklist

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
@jennuine jennuine added the QA Quality assurance. label Aug 25, 2022
@jennuine jennuine requested a review from ahcorde August 25, 2022 17:27
@jennuine jennuine self-assigned this Aug 25, 2022
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Aug 25, 2022
@codecov
Copy link

codecov bot commented Aug 25, 2022

Codecov Report

Merging #477 (d96237e) into ign-gui3 (0948063) will increase coverage by 0.30%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           ign-gui3     #477      +/-   ##
============================================
+ Coverage     73.51%   73.82%   +0.30%     
============================================
  Files            30       30              
  Lines          3270     3270              
============================================
+ Hits           2404     2414      +10     
+ Misses          866      856      -10     
Impacted Files Coverage Δ
src/plugins/key_publisher/KeyPublisher.cc 100.00% <0.00%> (+43.47%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

just a minor comment, LGTM

@jennuine
Copy link
Contributor Author

@ahcorde what's the minor comment?

Comment on lines +110 to +124
protected: Application app{g_argc, g_argv};

// Instance of the main window.
protected: MainWindow *win;

// List of plugins.
protected: QList<KeyPublisher *> plugins;
protected: KeyPublisher *plugin;

// Checks if a new key has been received.
protected: bool received = false;
protected: transport::Node node;

// Current key
protected: int currentKey = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

private instead of protected ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If private then the members won't be accessible, specifically we can't call:

this->VerifyKeyEvent(Qt::Key_W);
this->VerifyKeyEvent(Qt::Key_A);
this->VerifyKeyEvent(Qt::Key_D);

@jennuine jennuine requested a review from ahcorde November 4, 2022 17:02
@jennuine jennuine merged commit b09d084 into ign-gui3 Nov 7, 2022
@jennuine jennuine deleted the jennuine/key_pub_test branch November 7, 2022 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel QA Quality assurance.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants