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

issue77: Fixed the invalid range error issue with the small matchee raw topic when compared with base raw topic length #78

Merged
merged 4 commits into from
Feb 10, 2024

Conversation

ashu-pietech
Copy link
Contributor

Added another condition to check if matchee topic's fragments length is less than the length of base topic's fragment. Currently there's only one condition to handle the opposite condition such as if base topic's fragment length is less than matchee fragment length

…aw topic when compared with base raw topic length
@@ -95,6 +95,13 @@ class MqttSubscriptionTopic extends MqttTopic {
return false;
}
}
// If we're at the last fragment of the matchee rawTopic but there are
// more fragments in the in the lhs rawTopic then the matchee rawTopic
Copy link
Owner

Choose a reason for hiding this comment

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

'in the in the' typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I'll fix it

@shamblett
Copy link
Owner

Ok, the file mqtt_client_base_test.dart contains a group test ''Subscription Topic' to unit test this class, could you add a test/s to test your change please.

@ashu-pietech
Copy link
Contributor Author

Ok, the file mqtt_client_base_test.dart contains a group test ''Subscription Topic' to unit test this class, could you add a test/s to test your change please.

Sure I'll check that

@shamblett
Copy link
Owner

Re released the package for issue 79, you will probably need to rebase.

@ashu-pietech
Copy link
Contributor Author

@shamblett I've updated the PR after rebase

@shamblett
Copy link
Owner

OK an existing unit test is now failing, could you have a look please, thanks.

@ashu-pietech
Copy link
Contributor Author

OK an existing unit test is now failing, could you have a look please, thanks.

Okay I'll check it

@ashu-pietech
Copy link
Contributor Author

@shamblett PR updated and now all the test cases are working

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0f8e103) 61.23% compared to head (ff1f241) 61.18%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #78      +/-   ##
==========================================
- Coverage   61.23%   61.18%   -0.06%     
==========================================
  Files         102      102              
  Lines        3555     3558       +3     
==========================================
  Hits         2177     2177              
- Misses       1378     1381       +3     

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

@shamblett shamblett merged commit d034bad into shamblett:master Feb 10, 2024
2 of 3 checks passed
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