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

Tasty #237

Merged
merged 57 commits into from
Sep 29, 2019
Merged

Tasty #237

merged 57 commits into from
Sep 29, 2019

Conversation

robrix
Copy link
Collaborator

@robrix robrix commented Sep 29, 2019

This PR migrates tests & examples to use tasty, removes some of them, and rearranges some others.

  • Migrate the examples to use tasty.

@robrix robrix marked this pull request as ready for review September 29, 2019 20:42
Copy link
Collaborator Author

@robrix robrix left a comment

Choose a reason for hiding this comment

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

This is setting the stage for us to do thorough documentation & testing of effect laws ✨

@@ -0,0 +1,47 @@
{-# LANGUAGE FlexibleContexts, FlexibleInstances, GeneralizedNewtypeDeriving, MultiParamTypeClasses, TypeApplications, TypeOperators, UndecidableInstances #-}
module Inference
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I brought this over from the unit tests and expanded on it significantly to document the idiom.

inference :: Spec
inference = describe "inference" $ do
it "can be wrapped for better type inference" $
run (runHasEnv (runEnv "i" ((++) <$> askEnv <*> askEnv))) `shouldBe` "ii"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a pretty important technique and underserved here, so I moved it into the examples, above.


reinterpretation :: Spec
reinterpretation = describe "reinterpretation" $ do
it "can reinterpret effects into other effects" $
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. We’re testing this already in the actual library, e.g. with FreshC.

  2. This is not a good example; it’s contrived, and ill-justified.

Therefore, I’ve opted to 🔥 it since it’s not serving any purpose here and I’d rather not spend the time rewriting it into an example; this is honestly deserving of a much better-thought-out example (cf #9).



interposition :: Spec
interposition = describe "interposition" $ do
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. This exists in the library, so reimplementing it is particularly purposeless.

  2. It’s going away in 🔥 interposition #223 anyhow.

  3. It’s also the wrong way to do this.

  4. Plus there’s no motivation, it’s a bad example, etc.

🔥

shouldSucceed (Failure f) = expectationFailure f

fusion :: Spec
fusion = describe "fusion" $ do
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This has been moved into its own module.

@@ -1,23 +0,0 @@
module Control.Effect.NonDet.Spec
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved to NonDet.


failureOf :: Inspection.Result -> Maybe String
failureOf (Success _) = Nothing
failureOf (Failure f) = Just f
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I really wanted to be able to use @?=, so I made this just return the String. We’ll see the error message in failures.

import Test.Tasty.HUnit

tests :: TestTree
tests = testGroup "NonDet"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I really wanted to abstract these over the carrier so that we could test them properly against each carrier independently, but the ridiculous contortions that required were not worth the potential benefit.

New plan: can we express interpreting an effect as an effect? 🤔

@@ -0,0 +1,13 @@
module Main
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed this file while I was at it. I don’t like the word “spec” for “test.”

@robrix robrix merged commit 0ca3d54 into master Sep 29, 2019
@robrix robrix deleted the tasty branch September 29, 2019 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants