Skip to content
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

Recipe coverage graph test #2030

Merged
merged 8 commits into from
Jul 13, 2024
Merged

Recipe coverage graph test #2030

merged 8 commits into from
Jul 13, 2024

Conversation

xsebek
Copy link
Member

@xsebek xsebek commented Jul 13, 2024

  • Use the recipe graph to find unreachable entities instead of only looking for recipes for devices.
  • Add yielded entities in the recipe graph
  • Add a test for each entity.

Before:

   Recipe coverage
    Ensure all devices have recipes (#1268):                                            FAIL (expected: Need to come up with more recipes)
      test/unit/TestRecipeCoverage.hs:36:
      Missing recipes for: "Elmer's glue", "binoculars", "blueprint", "caliper", "decoder ring", "dozer blade", "hourglass", "lambda", "linotype", "olfactometer", "rolex", "tape drive", and "wedge" (expected failure)

After

  Ensure all entities have recipes
    3D printer:                   OK
    ADT calculator:               OK
    Elmer's glue:                 FAIL (expected: More recipes needed (#1268))
      test/integration/TestRecipeCoverage.hs:34:
      Can not make "Elmer's glue" from starting entities. (expected failure)
    [... 100 more entities ...]

@xsebek xsebek requested review from kostmo and byorgey July 13, 2024 17:26
@noahyor
Copy link
Member

noahyor commented Jul 13, 2024

Looks good, but I don't really know enough about this to review it.

@xsebek
Copy link
Member Author

xsebek commented Jul 13, 2024

I am almost tempted to create a test for each entity - that way, when an entity is added without a recipe we would know.

That's what originally made me check out the test. Then I realized it is a simpler version of a recipe graph and only for devices, so I refactored it and got a few extra entities in the list.

@xsebek
Copy link
Member Author

xsebek commented Jul 13, 2024

And so I generated 100 more tests:

Screenshot 2024-07-13 at 20 43 24

@noahyor you said in #2025 that you expected the tests to fail - if this is merged before #2028, then they will. 😉

@xsebek xsebek requested a review from noahyor July 13, 2024 18:47
Copy link
Member

@noahyor noahyor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think it's weird that it says the lambda doesn't have a recipe.

@noahyor
Copy link
Member

noahyor commented Jul 13, 2024

@noahyor you said in #2025 that you expected the tests to fail - if this is merged before #2028, then they will. 😉

Well, there you go!

@xsebek
Copy link
Member Author

xsebek commented Jul 13, 2024

I still think it's weird that it says the lambda doesn't have a recipe.

Well, it does not. (It is only used to make curry.) That's why the test that @kostmo wrote reported it.

But with a recipe graph, the entities found in the world and base inventory are taken as the starting point, and everything else has to be craftable from them. So lambda is no longer reported. 🙂

@xsebek
Copy link
Member Author

xsebek commented Jul 13, 2024

Ha, it seems I did not fetch main:

hourglass:                                              OK (unexpected: More recipes needed (#1268))
   (unexpected success: More recipes needed (#1268))
    Use -p '/hourglass/' to rerun this test only.

@byorgey
Copy link
Member

byorgey commented Jul 13, 2024

tea leaves are obtainable from the world --- notice that they are yielded by a tea plant.

test/integration/TestRecipeCoverage.hs Outdated Show resolved Hide resolved
test/integration/TestRecipeCoverage.hs Outdated Show resolved Hide resolved
@xsebek
Copy link
Member Author

xsebek commented Jul 13, 2024

Thanks, @byorgey; it seems the recipe graph was not updated with the yields feature.

The infinite improbability drive is now properly shown as the hardest to obtain entity:
graphviz

@byorgey
Copy link
Member

byorgey commented Jul 13, 2024

Huh, strange, I thought I did update it a little while ago. Maybe I am thinking of something else.

Comment on lines -227 to 233
let yieldPairs = mapMaybe (\e -> (e ^. entityName,) <$> (e ^. entityYields)) . Map.elems $ entitiesByName emap
let yieldPairs = mapMaybe (\e -> (e ^. entityName,) <$> (e ^. entityYields)) . toList $ rgAllEntities graphData
mapM_ (uncurry (.->.)) (both getE <$> yieldPairs)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@byorgey I think you updated the graph display to Dot, but not the levels computation.

If Dot could lay out the graph well, the levels would not even be needed. But because we have so many connections, the levels at least group together nodes in order.

@xsebek xsebek added the merge me Trigger the merge process of the Pull request. label Jul 13, 2024
@mergify mergify bot merged commit 9504996 into main Jul 13, 2024
12 checks passed
@mergify mergify bot deleted the recipe-coverage-graph branch July 13, 2024 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants