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

[rest] Allow colons in SSE topics filter #1052

Merged
merged 1 commit into from
Sep 29, 2019
Merged

[rest] Allow colons in SSE topics filter #1052

merged 1 commit into from
Sep 29, 2019

Conversation

falmanna
Copy link
Contributor

fixes #1043

@cweitkamp
Copy link
Contributor

Thanks. Can you please sign-off your work?

It would be nice to have some unit tests for the new pattern to cover at least all available core events. May I ask you to add them?

@cweitkamp cweitkamp added bug An unexpected problem or unintended behavior of the Core REST/SSE labels Sep 28, 2019
@falmanna falmanna changed the title [rest] allow colons in sse topics filter [rest] Allow colons in sse topics filter Sep 29, 2019
@falmanna
Copy link
Contributor Author

I signed off the work and added some tests for the single entity filter.

@cweitkamp
Copy link
Contributor

cweitkamp commented Sep 29, 2019

Unfortunately the test failed https://travis-ci.org/openhab/openhab-core/jobs/591061769#L2130 - It looks like the dash ("-") is not allowed too:

[ERROR] Tests run: 4, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.24 s <<< FAILURE! - in org.eclipse.smarthome.io.rest.sse.internal.util.SseUtilTest
[ERROR] testValidInvalidFilters(org.eclipse.smarthome.io.rest.sse.internal.util.SseUtilTest)  Time elapsed: 0.139 s  <<< FAILURE!
java.lang.AssertionError: 

Expected: is <true>
     but: was <false>
	at org.eclipse.smarthome.io.rest.sse.internal.util.SseUtilTest.testValidInvalidFilters(SseUtilTest.java:56)

[ERROR] Failures: 
[ERROR]   SseUtilTest.testValidInvalidFilters:56 
Expected: is <true>
     but: was <false> 
[ERROR] Tests run: 4, Failures: 1, Errors: 0, Skipped: 0

@falmanna
Copy link
Contributor Author

That's strange, why did it pass when I ran it locally in Eclipse?

@falmanna
Copy link
Contributor Author

Okay, apparently I didn't rebuild the bundle before running tests.

Should I add another commit? or modify the previous?

Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

LGTM. And thank you very much for your first contribution over here.

@cweitkamp cweitkamp merged commit b48f1c1 into openhab:master Sep 29, 2019
@cweitkamp cweitkamp added this to the 2.5 milestone Sep 29, 2019
@wborn wborn changed the title [rest] Allow colons in sse topics filter [rest] Allow colons in SSE topics filter Dec 8, 2019
Rosi2143 pushed a commit to Rosi2143/openhab-core that referenced this pull request Dec 26, 2020
Removed structure from binding page as it's duplicated on the guidelines page.
Updated and improved the description on the guidelines page with the information that was on the bindings page (that information was improved recently).

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of the Core REST/SSE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REST] Subscribing to single thing endpoint not working
2 participants