backend/src/test/java/org/booklore/service/metadata/parser/LubimyCzytacParserTest.java (2)

185-187: Brittle exact-length assertion may fight future fixture maintenance.

assertEquals(827, result.getDescription().length()) couples the test to the precise whitespace/HTML structure of example-book.html. Any unrelated trim/normalize change in the parser (or a fixture re-save that touches whitespace) will require updating the magic number. Consider replacing with a loose lower-bound check (e.g., assertTrue(result.getDescription().length() > 500)) plus the existing prefix assertion, which is what the comment "in the right ballpark" suggests anyway.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/src/test/java/org/booklore/service/metadata/parser/LubimyCzytacParserTest.java`
around lines 185 - 187, The test in LubimyCzytacParserTest is too brittle
because it asserts an exact description length with assertEquals(827,
result.getDescription().length()); change this to a loose bound check to
tolerate harmless whitespace/HTML changes: remove or replace the assertEquals on
result.getDescription().length() with an assertion like
assertTrue(result.getDescription().length() > 500) (or another sensible lower
bound) while keeping the existing prefix check assertEquals("Jedno z
najoryginalniejszych", result.getDescription().substring(0, 28)) to ensure
content is in the right ballpark.

---

191-195: Replace assert with an explicit null check.

JVM assertions are off by default; if lubimyczytac/example-search.html or lubimyczytac/example-book.html is ever missing from the test classpath, you'll get a confusing NPE on is.readAllBytes() rather than a clear failure pointing at the missing fixture. Surefire typically enables -ea, but relying on that is fragile — prefer Objects.requireNonNull(is, filename) or an explicit throw new FileNotFoundException(filename).

🔧 Proposed fix

     private String readResourceFile(String filename) throws IOException {
         try (InputStream is = getClass().getClassLoader().getResourceAsStream(filename)) {
-            assert is != null;
-
-            return new String(is.readAllBytes());
+            if (is == null) {
+                throw new IOException("Test fixture not found on classpath: " + filename);
+            }
+            return new String(is.readAllBytes(), StandardCharsets.UTF_8);
         }
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/src/test/java/org/booklore/service/metadata/parser/LubimyCzytacParserTest.java`
around lines 191 - 195, Replace the fragile JVM assertion in the
resource-loading block with an explicit null check: in the method that reads the
test fixture (where InputStream is =
getClass().getClassLoader().getResourceAsStream(filename) is used in
LubimyCzytacParserTest), detect if is == null and throw a clear exception (e.g.,
new FileNotFoundException(filename) or use Objects.requireNonNull(is, filename))
before calling is.readAllBytes(), so missing test resources produce an explicit,
informative failure rather than a NPE.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

backend/src/test/java/org/booklore/service/metadata/parser/LubimyCzytacParserTest.java (2)

146-153: Mismatched Book.title vs. request title is confusing.

Book is built with "Wiedźmin" while the FetchMetadataRequest uses "Sklepy cynamonowe". The parser drives its search off the request title, so the test still works, but the leftover "Wiedźmin" reads like a copy/paste from earlier iterations and could mislead future readers debugging the fixture flow.

✏️ Proposed cleanup

         Book book = Book.builder()
-            .title("Wiedźmin")
+            .title("Sklepy cynamonowe")
             .build();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/src/test/java/org/booklore/service/metadata/parser/LubimyCzytacParserTest.java`
around lines 146 - 153, In LubimyCzytacParserTest update the test fixture so the
Book's title matches the FetchMetadataRequest title to avoid confusion: change
the Book built via Book.builder().title("Wiedźmin") to use the same title string
as the request (e.g., "Sklepy cynamonowe") or otherwise use a clearly labeled
placeholder; ensure the variables referenced (Book.builder(), the Book instance,
and FetchMetadataRequest.builder() / request) are consistent so readers see that
the parser is driven by request.title.

---

198-212: Reusing a single parsed Document across all book-detail fetches.

mockResponse parses response once and stubs mockConnection.get() to always return that same Document instance. In parseBookDetails, the loop in fetchMetadata invokes fetchWithRetry for every URL returned from search, so every iteration receives the same Document. That works for the current single-result assertions, but:

• results will contain duplicates (one per search hit, capped at MAX_RESULTS), which is misleading if anyone later asserts on results.size().
• Jsoup's Document is mutable; if any future parser logic mutates the DOM (e.g., .remove(), .unwrap()), state would leak across iterations and tests.

Consider parsing the fixture fresh on each get() call (e.g., have the stub return Jsoup.parse(response) lazily via thenAnswer) so each invocation gets an independent Document.

♻️ Proposed refactor

-        // There may be a better way to get the parse to work here.
-        // However, this was the quickest and simplest way I could find.
-        mockJsoup.when(() -> Jsoup.parse(response)).thenCallRealMethod();
-        Document document = Jsoup.parse(response);
-
-        when(mockConnection.get()).thenReturn(document);
+        // Re-parse on each call so iterations don't share mutable DOM state.
+        mockJsoup.when(() -> Jsoup.parse(response)).thenCallRealMethod();
+        when(mockConnection.get()).thenAnswer(inv -> Jsoup.parse(response));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/src/test/java/org/booklore/service/metadata/parser/LubimyCzytacParserTest.java`
around lines 198 - 212, The test reuses a single mutable Jsoup Document by
stubbing mockConnection.get() to always return the same parsed instance in
mockResponse, causing duplicated/tainted state across fetchWithRetry iterations
invoked by fetchMetadata/parseBookDetails; change the stub to lazily return a
fresh parse each call (replace when(mockConnection.get()).thenReturn(document)
with a thenAnswer that returns Jsoup.parse(response) on each invocation) and
remove or adjust the earlier mockJsoup.parse stubbing so each get() yields an
independent Document.