-
Notifications
You must be signed in to change notification settings - Fork 24
spring boot retrofit + gradle addition & general cleanup / fixes #203
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
Conversation
…ing against superconductor
…ribute and downstream dependents
issue #204 (ElementAttribute "nip" field) addressed with changes included as part of this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In test cases testNIP01SendTextNoteEventGeoHashTag
, testNIP01SendTextNoteEventHashtagTag
, testNIP01SendTextNoteEventCustomGenericTag
, testFiltersListReturnSameSingularEvent
, testFiltersListReturnTwoDifferentEvents
, and testMultipleFiltersDifferentTypesReturnSameEvent
, the published events are never purged from the relay, and the expression assertEquals(2, result.size());
will always fail if you run the test cases more than once.
To prevent this, you have to make the content unique, for example, I am introducing the index
variable like this, to ensure that the variables geoHashTagTarget
and genericTagTarget
are unique:
@Test
public void testMultipleFiltersDifferentTypesReturnSameEvent() throws IOException {
System.out.println("testMultipleFilters");
Long index = System.currentTimeMillis();
String geoHashTagTarget = "geohash_tag-location-DifferentTypesReturnSameEvent" + index;
GeohashTag geohashTag = new GeohashTag(geoHashTagTarget);
String genericTagTarget = "generic-tag-value-DifferentTypesReturnSameEvent" + index;
GenericTag genericTag = GenericTag.create("m", genericTagTarget);
NIP01<NIP01Event> nip01 = new NIP01<>(Identity.generateRandomIdentity());
nip01.createTextNoteEvent(List.of(geohashTag, genericTag), "Multiple Filters").signAndSend(relays);
Filters filters = new Filters(
new GeohashTagFilter<>(new GeohashTag(geoHashTagTarget)),
new GenericTagQueryFilter<>(new GenericTagQuery("#m", genericTagTarget)));
List<String> result = nip01.sendRequest(filters, UUID.randomUUID().toString());
assertFalse(result.isEmpty());
assertEquals(2, result.size());
assertTrue(result.stream().anyMatch(s -> s.contains(geoHashTagTarget)));
// nip01.close();
}
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, yes- considering some common testing-related semantics:
-
integration testing: definition/practice/expectation is for a system under test to have predefined/known state before tests are started, including any/all data. existing nostr-java (and SC) integration tests are implemented as per this definition.
-
end-to-end/functional testing: aligns with what you're describing (tests using persistent/ongoing data/states) as conventional/practice. i'm open to whatever semantics you prefer, but given the above definition- we haven't (yet) implemented a testing scenario as you've described.
if helpful/useful, i've just pushed an automated integration-test bash script into sc_spring_boot-gradle-migration branch that might help streamline/simplify your integration testing process.
prior to use, note the requisite configuring relevant nostr-java & SC directories
w/ abbreviated main logic as follows:
- cd to nostr-java
- rm m2_local
- build_nostr-java
- publish nostr-java to m2_local
- cd to superconductor
- build and test super
- start super_service
- run nostr-java tests
- terminate super
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
supplemental, if integration testing with SC as your relay server: as SC develop branch is currently several commits ahead of nostr-java PR branch, you'll either want to use:
- SC commit 614686786e7009b2afcd989a4a1e76159b3ecd64
or
- nostr-java develop-branch compatible sc_spring_boot-gradle-migration branch, which includes both autotest.sh script (+ newly added maven option) and maven failsafe retry activated for "flaky" tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testNIP99CalendarContentPreRequest
fails on my machine.
assertTrue(
JsonComparator.isEquivalentJson(
MAPPER_AFTERBURNER.readTree(expected),
MAPPER_AFTERBURNER.readTree(reqResponse)));
org.opentest4j.AssertionFailedError: expected: but was:
at app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at app//org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:31)
at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:183)
at app//nostr.api.integration.ApiNIP52RequestIT.testNIP99CalendarContentPreRequest(ApiNIP52RequestIT.java:142)
at java.base@21.0.6/java.lang.reflect.Method.invoke(Method.java:580)
at java.base@21.0.6/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:387)
at java.base@21.0.6/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1312)
at java.base@21.0.6/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1843)
at java.base@21.0.6/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1808)
at java.base@21.0.6/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:188)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gotcha, can you post/send the test-command executed and full stacktrack/debug-output for that test class?
somewhat related if you haven't yet encountered- any integration test class using websockets are necessarily multi-threaded with multiple concurrent websocket connections. this can/may lead to concurrency states for both websocket connections and database state and as a result, it is not uncommon for a test method to fail. these failures are false-positives and rectifiable by activating (uncommenting) gradle's retry pass
@tcheeric hi, eric. just a heads up if using autotest.sh maven mode, its been updated to properly terminate maven child threads during integration testing shutdown. gradle did not require any changes. |
Ok, noted. I will update my SC. Thanks Nick |
good catch, yes- looks like those two files were missed when merging ElementAttribute related changes from my in-progress/develop branch. the fix changes you made are exactly correct and are now also be present in sc_spring_boot-gradle-migration commit cd04955581b2f0b30a7178247fd65ef848adb9e1, apologies! commit cd04955581b2f0b30a7178247fd65ef848adb9e1 (HEAD -> origin/sc_spring_boot-gradle-migration)
Author: nick avlo <avlo@yahoo.com>
Date: Wed Mar 26 18:38:05 2025 -0700
removed leftover NIP parameters |
@tcheeric hi, eric. saw your approval, super- & prior to merging into develop, just confirming you are/were able to:
if not, def lmk and i'll get to addressing either/both as necessary. |
Hi Nick,
Overall, the gradle integration is extremely useful. Thanks |
incorrect. autotest is a comprehensive unit-and-integration test suite for both SC and nostr- java.
IT db is a self-contained, transient, h2 in-memory db only. it runs separate from and independent of other external/standalone db you might have running. files:
are your friends.
strongly advise study towards fluency with your code base. i'm here to help, lmk if any questions.
ok great, will merge it 👍 |
Ok, great. I will review again tomorrow. |
@tcheeric. hi, eric. as previously discussed- below PR continuation of
develop
branch- comprised of project modernization, fixes & general cleanup sweep, as follows:module-info.java
files/handling no longer needed/necessary, thus removed. module dependencies now auto-management by spring.new ObjectMapper
replaced by 3 instances of compile-time-byte-code generated afterburnernull
use(abuse!) refactored intoOptional
for performance increase and resiliency...pom
transitive module dependency redundancies have been cleaned upnoteworthy:
(however, some IDE's will have trouble switching between maven/gradle context. easiest/common way to accommodate both is with a 2nd local copy of nostr-java repo, which can then be opened as a separate (gradle) project.)
gradle quickstart/reference:
after installing gradle, 1st time through gradle build will perform preliminary setup and may take some time. subsequent gradle tasks should run quickly thereafter
useful/frequent gradle commands (w/ maven equivalents)
note: as per convention/utility in many test frameworks (including junit & utilized by spring-test), unit test class names suffixed as *Test.java and integration tests suffixed *IT.java are automatically/cleanly contextualized, resourced and executed by the test framework (junit). thus, relevant nostr-java classes named *Test.java which are in fact integration tests- have been accordingly renamed as *IT.java.
as before, this functionality is transparent to whomever is running tests. maven/gradle build, test & publish commands should functional as usual/expected for the developer.
any questions/issues/etc, def lmk