Core tests #1162
Core tests #1162
Conversation
wcekan
force-pushed
the
coreTests
branch
2 times, most recently
from
77b476c
to
a8294ec
Compare
wcekan
force-pushed
the
coreTests
branch
6 times, most recently
from
35b80d7
to
75d8630
Compare
wcekan
force-pushed
the
coreTests
branch
3 times, most recently
from
b8b3eaa
to
21265e8
Compare
| } catch (IOException e) { | ||
| throw new UncheckedIOException(e); | ||
| } | ||
| if (doc2.getData() == null) { |
There was a problem hiding this comment.
See #1163. Moved there.
| @@ -110,6 +117,7 @@ public PersistentResourceTest() { | |||
| badUserScope = new RequestScope(null, null, mock(DataStoreTransaction.class), | |||
| new User(-1), null, elideSettings); | |||
| reset(goodUserScope.getTransaction()); | |||
| assertFalse(goodUserScope.isUseFilterExpressions()); | |||
There was a problem hiding this comment.
This is not a test. Why use assert here?
There was a problem hiding this comment.
Why is this not a test? Will move.
| @Test | ||
| public void testPatchRequestScope() { | ||
| DataStoreTransaction tx = mock(DataStoreTransaction.class); | ||
| PatchRequestScope parentScope = |
There was a problem hiding this comment.
Does the code actually nest patch scopes like this? I thought patch was the parent scope - but child scopes were always normal request scopes.
There was a problem hiding this comment.
Yes, they are nested. A new wrapper is generated for each operation in the patch extension. That is what this tests. See
| dictionary.bindTrigger(Book.class, OnUpdatePreCommit.class, onUpdateDeferredCallback, true); | ||
| dictionary.bindTrigger(Book.class, OnUpdatePreSecurity.class, onUpdateImmediateCallback, true); | ||
| dictionary.bindTrigger(Book.class, OnUpdatePostCommit.class, onUpdatePostCommitCallback, true); | ||
| dictionary.bindTrigger(Author.class, OnUpdatePostCommit.class, onUpdatePostCommitAuthor, true); | ||
| // enable trace | ||
| Logger rootLogger = (Logger) LoggerFactory.getILoggerFactory().getLogger(Logger.ROOT_LOGGER_NAME); | ||
| rootLogger.setLevel(Level.TRACE); |
There was a problem hiding this comment.
Do we want to leave this log level on?
There was a problem hiding this comment.
Yes, otherwise we are not testing logging works without throwing exceptions.
Otherwise you need to duplicate all these same tests except with logging on.
Description
Adding some missing tests.
Motivation and Context
Better elide-core code coverage
How Has This Been Tested?
Unit testing
License
I confirm that this contribution is made under an Apache 2.0 license and that I have the authority necessary to make this contribution on behalf of its copyright owner.