Cleanup Merlin Code #841
Merged
Cleanup Merlin Code #841
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mythicaeda
added
the
refactor
Mythicaeda
changed the title
Mythicaeda
force-pushed
the
refactor/remove-vestigal-classes-merlin-server
branch
from
4cb174e
to
96940a2
Compare
mattdailis
reviewed
Apr 4, 2023
...er/src/main/java/gov/nasa/jpl/aerie/merlin/driver/timeline/RecursiveEventGraphEvaluator.java
Outdated
Show resolved
Hide resolved
Mythicaeda
force-pushed
the
refactor/remove-vestigal-classes-merlin-server
branch
from
96940a2
to
108d249
Compare
camargo
approved these changes
Apr 18, 2023
There was a problem hiding this comment.
LGTM based on your overview yesterday. Let's get sign off from @mattdailis as well.
mattdailis
approved these changes
Apr 18, 2023
* Replaced with Plan.java, which has identical behavior but tracked ActivityDirectiveIds * Renamed InMemoryPlanRepository.createPlan to InMemoryPlanRepository.storePlan. Appears to be more accurate based on behavior with the replacement of NewPlan with Plan
- CreatedEntity - NewMissionModel - ActivityTransaction - PostgresPlanTransaction - AssociatePlanDatasetAction - CreateDatasetPartitionsAction - UpdatePlanAction - Window - BreadcrumbCursor - DevAppDriver - Fixtures - TaskSource
- NoSuchActivityInstanceException - UnexpectedMissingPlanException - UnexpectedMissingMissionModelException - UnexpectedJSONException - NewMissionModelValidationException
- PostrgesMissionModelRepository.GetUnusedFilename - PostgresResultsCellRepository.getPlan - PlanRepository.getAllActivitiesInPlan
- Remove unneeded variable from PostgresPlanRepository.createPlanDataset - Remove redundant 'final' modifier from ProfileRecord and InMemoryRevisionData - Simplify 'if' in InMemoryCell.failWith - Convert Worker into a record - Remove unused import in SynchronousSimulationAgent, EffectExpressionDisplay, and CellRef - Convert statement lambdas to expression lambdas in MerlinBindings - Remove unthrown exceptions from InMemoryMissionModelRepository - Simplify SchedulingInstant.compareTo - Rename activityInstanceIdP to activityDirectiveIdP in MerlinParsers - Remove unused empty constructor for IntegrationFailureException - Remove isColumnNull() from GetModelConstraintsAction, GetPlanConstraintsAction, and GetSpanRecords
- Breadcrumb was only used by ValidationException, which has been moved to the parsers.
Its parent class had no other inheritors, so these tests were also effectively disabled
Deletion of Simulation Datasets should be initiated by the user. User defined automation of deletion should be done through the Aerie API, not Aerie Merlin
- Remove other unused code in MerlinBindingsTest - Remove FakeFile, which is unused as of MerlinBindingsTest changes
The InMemory versions of the repositories are either stubs or only defined enough to be used in testing. Therefore, we should remove the ability to initiate Aerie using an InMemoryStore. - Remove classes that are unused as of this pruning
Mythicaeda
force-pushed
the
refactor/remove-vestigal-classes-merlin-server
branch
from
108d249
to
c082e8c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Read the commit history for a detailed breakdown of exactly what was removed or changed. Overall, however, this PR:
merlin-server,merlin-driver, andmerlin-worker.InMemoryMissionModelRepositoryisn't used in testing, nearly all of its methods are stubs.Verification
Tests that didn't test live code were removed, as well as tests that have been disabled since November 2021.
#840 was created to address a gap I noticed in
MerlinBindingsTests.#871 was created to address a gap noticed in
EventGraphEvaluator.Documentation
No documentation changes should be necessary.
Future work
Most of the
InMemory_code is only used in tests ofInMemorycode, rather than being used to avoid having unit tests of things like theSimulationDriverturn into e2eTests. I'd like to take a finer tooth comb and see if we can either drop these classes entirely or actually use them in tests of live Merlin code.