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

GEODE-1296: change conditional in getRawBytes to assert #145

Closed
wants to merge 3 commits into from

Conversation

pdxrunner
Copy link

Made suggested change to getRawBytes. Updated corresponding unit test.
See review request https://reviews.apache.org/r/47479/

@dschneider-pivotal
Copy link
Contributor

I think this should be changed to use the java "assert" keyword. The assertion should only be checked when assertions on the jvm are enabled.

Fixed unit tests that spy getRawBytes that failed after making the above
change.
protected byte[] getRawBytes() {
assert !isCompressed();
byte[] result = getCompressedBytes();
Copy link
Contributor

Choose a reason for hiding this comment

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

Combine these two lines into "return getCompressedBytes();"

Made suggested changes and added @BeforeClass and @afterclass methods to
enable Java assertions for testing, and restor the original state at the end
of the test.
@pdxrunner
Copy link
Author

Precheckin passed with my most recent update. I think this one is ready to go now.

@dschneider-pivotal
Copy link
Contributor

The travis failure is a know intermittent issue in a new unit test.
This change looks good. I will pull it in.

@pdxrunner
Copy link
Author

This was checked in by dschneider. Closing the PR

@pdxrunner pdxrunner closed this May 25, 2016
@pdxrunner pdxrunner deleted the feature/GEODE-1296 branch August 26, 2016 19:59
robbadler pushed a commit to smgoller/geode that referenced this pull request Jul 12, 2022
Changed several tests to make them stop spying concrete JDK classes. In
Java 17, the packages of these classes are not open for reflection,
causing Mockito to create spies that are incomplete and unusable.`

Changed `ClientRegistrationEventQueueManagerTest` to subclass and
override `java.util.concurrent.locks.ReentrantReadWriteLock`
and`java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock`.

Changed `OverflowOplogFlushTest` to wrap the
`java.nio.channels.FileChannel` instances in decorator classes that
intercept and modify the channels' behavior.

Changed `StartMemberUtilsTest` to use `mock(java.io.File.class)` instead
of spying an existing instance.

Even with these changes, `geode-test.gradle` must open these packages to
tests, because `Jetty9CachingClientServerTest` still needs them to be
opened. That will be fixed on a separate PR.
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.

2 participants