Skip to content

CHANGE (OTHER): @W-14573108@: Fix cli-messaging tests and make subprojects use same gradle version as root #1254

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 30, 2023
Merged

CHANGE (OTHER): @W-14573108@: Fix cli-messaging tests and make subprojects use same gradle version as root #1254

merged 3 commits into from
Nov 30, 2023

Conversation

@stephen-carter-at-sf
Copy link
Contributor

@stephen-carter-at-sf stephen-carter-at-sf commented Nov 29, 2023

No description provided.

@stephen-carter-at-sf stephen-carter-at-sf changed the title @W-14573108@ Fix cli-messaging tests and make subprojects use same gradle version as root CHANGE(OTHER): @W-14573108@ Fix cli-messaging tests and make subprojects use same gradle version as root Nov 29, 2023
@stephen-carter-at-sf stephen-carter-at-sf changed the title CHANGE(OTHER): @W-14573108@ Fix cli-messaging tests and make subprojects use same gradle version as root CHANGE (OTHER): @W-14573108@: Fix cli-messaging tests and make subprojects use same gradle version as root Nov 29, 2023
jfeingold35
jfeingold35 reviewed Nov 29, 2023
View reviewed changes
cli-messaging/src/test/java/com/salesforce/messaging/EventKeyTest.java Outdated

private EventKey eventKey;
public static Stream<EventKey> getAllInfoEventKeyValues() {
return getAllEventKeyValues().filter(eventKey -> eventKey.getMessageType() == MessageType.INFO);
Copy link
Contributor

@jfeingold35 jfeingold35 Nov 29, 2023

Choose a reason for hiding this comment

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

Why do this method and getAllWarningEventKeyValues() filter by getMessageType() == instead of getMessageKey().startsWith()? As is, since the tests themselves check getMessageType() they're basically just checking A == A.

Copy link
Contributor Author

@stephen-carter-at-sf stephen-carter-at-sf Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Notice that INFO_TELEMETRY starts with "info." and yet it isn't of MessageType.INFO ... it is MessageType.TELEMETRY. That's why I chose to use the MessageType wheenever possible instead of the naming convention. Thoughts?

Oh wait I see what you mean now... the assertion for MessageType.INFO is kind of useless now. What do you want to do with the INFO_TELEMETRY then? Should we rename it so that it isn't under info.* but instead under something like telementry.*?

Copy link
Contributor Author

@stephen-carter-at-sf stephen-carter-at-sf Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed. Just updated the test for now.

@jfeingold35 jfeingold35 merged commit 663b323 into dev Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jfeingold35 jfeingold35 jfeingold35 approved these changes

Assignees

No one assigned

Labels

cla:signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@stephen-carter-at-sf @jfeingold35