Skip to content

Conversation

@ascopes
Copy link
Owner

@ascopes ascopes commented Feb 5, 2023

A bug was discovered that results in a NoSuchElementException being raised if a user attempts to register a module in an output location that was created without an initial package on a file system to place files in.

To fix this bug, a few core functionalities have changed slightly:

  • JctFileManager can now throw IllegalArgumentException from createEmptyLocation(). This will be thrown if the location is an output location or a module location.
  • OutputContainerGroupImpl will now raise an IllegalStateException if it detects that the container group has been misconfigured as outlined above. This error will explain how to mitigate the issue.
  • ContainerGroupRepositoryImpl will now register the path root that resides behind one or more modules being registered to it. This is also covered by the case that will check if no modules were discovered in anything being registered, since this will treat the path as an empty single package root anyway.
  • ContainerGroupRepositoryImpl will now refuse to create empty locations for output locations, as this would not make sense under the above constraints.

A bug was discovered that results in a NoSuchElementException being raised
if a user attempts to register a module in an output location that was
created without an initial package on a file system to place files in.

To fix this bug, a few core functionalities have changed slightly:

- JctFileManager can now throw IllegalArgumentException from
  createEmptyLocation(). This will be thrown if the location is
  an output location or a module location.
- OutputContainerGroupImpl will now raise an IllegalStateException
  if it detects that the container group has been misconfigured as
  outlined above. This error will explain how to mitigate the issue.
- ContainerGroupRepositoryImpl will now register the path root that
  resides behind one or more modules being registered to it. This
  is also covered by the case that will check if no modules were
  discovered in anything being registered, since this will treat
  the path as an empty single package root anyway.
- ContainerGroupRepositoryImpl will now refuse to create empty
  locations for output locations, as this would not make sense
  under the above constraints.
@ascopes ascopes added bug Something isn't working 👕 small labels Feb 5, 2023
@ascopes ascopes added this to the First Stable Release (v0.0.1) milestone Feb 5, 2023
@ascopes ascopes self-assigned this Feb 5, 2023
@github-actions
Copy link

github-actions bot commented Feb 5, 2023

Test Results

  1 544 files  ±0    1 544 suites  ±0   0s ⏱️ ±0s
21 276 tests ±0  21 264 ✔️ ±0  12 💤 ±0  0 ±0 
21 540 runs  ±0  21 528 ✔️ ±0  12 💤 ±0  0 ±0 

Results for commit 4a2f8c5. ± Comparison against base commit 8c583ff.

@codecov-commenter
Copy link

Codecov Report

Merging #326 (4a2f8c5) into main (8c583ff) will decrease coverage by 0.03%.
The diff coverage is 53.84%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #326      +/-   ##
==========================================
- Coverage   78.86%   78.84%   -0.03%     
==========================================
  Files          90       90              
  Lines        2697     2699       +2     
  Branches      225      225              
==========================================
+ Hits         2127     2128       +1     
- Misses        514      515       +1     
  Partials       56       56              
Impacted Files Coverage Δ
.../jct/containers/impl/OutputContainerGroupImpl.java 48.83% <33.33%> (-1.17%) ⬇️
.../containers/impl/ContainerGroupRepositoryImpl.java 58.01% <56.52%> (+0.43%) ⬆️

@ascopes ascopes merged commit 61f6fa7 into main Feb 5, 2023
@ascopes ascopes deleted the bugfix/empty-output-location-case branch February 5, 2023 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants