-
Notifications
You must be signed in to change notification settings - Fork 70
Test cases for retrieveProgram #1609
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
base: master
Are you sure you want to change the base?
Test cases for retrieveProgram #1609
Conversation
import Controllers.Program (index, retrieveProgram) | ||
import qualified Data.ByteString.Lazy.Char8 as BL | ||
import Data.List (isInfixOf) | ||
import Data.Time.Clock (getCurrentTime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change in import here is okay, but you shouldn't change the order (previously this was included under the Data.Text
import
retrieveprogramTestCases = | ||
[ ("Valid program code returns JSON" | ||
, ["ASMAJ1689"] | ||
, T.pack "ASMAJ1689" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The T.pack
is unnecessary. We should have a compiler flag turned on so that any string literal is automatically inferred to have the correct type.
|
||
response <- runServerPartWithProgramQuery Controllers.Program.retrieveProgram (T.unpack queryParam) | ||
let actual = BL.unpack $ rsBody response | ||
case expected of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This structure is okay but should be simplified. In all of the non-error cases, you can (and should) be asserting a match against the exact expected string, not just an expected substring.
In the non-error case, the current test is quite broad (not (null actual))
). This is again a place where you should be asserting the exact error message.
insertProgram code = do | ||
curr <- liftIO getCurrentTime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed testTimestamp
to enable exact JSON string matching in tests. This is a
temporary solution to satisfy the requirement.
Is there any better testing approach I should take that can check the exact time in the JSON response?
Proposed Changes
Added test cases for the
retrieveProgram
method ofapp/Controllers/Program.hs
inbackend-test/Controllers /ProgramControllerTests.hs
, covering the cases:Added a helper function in
backend-test/TestHelpers.hs
to runServerPartWithProgramQuery
Response forretrieveProgram
.Modified
insertPrograms
helper - It now uses a fixedtestTimestamp
instead ofgetCurrentTime
to ensure consistent JSON output for exact matchingUsing a fixed
testTimestamp
ininsertPrograms
is a temporary solution.Type of Change
(Write an
X
or a brief description next to the type or types that best describe your changes.)Checklist
(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the
[ ]
into a[x]
in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)Before opening your pull request:
After opening your pull request: