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.

Sign up for GitHub

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

Rename and fix unit test target #47

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Rename and fix unit test target #47

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5ba08cb
Rename and fix unit test target
aronerben Aug 15, 2022
d24f718
Remove lwt dependency in unit tests, add alcotest
aronerben Aug 15, 2022
796b4c4
Remove CI cache
aronerben Aug 15, 2022
df2ce16
Update CI setup
aronerben Aug 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ jobs:
with:
ocaml-version: ${{ matrix.ocaml-version }}
- name: Install dependencies
if: steps.cache-opam.outputs.cache-hit != 'true'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mikonieminen @joseferben This line prevents alcotest from being installed, so I deleted it. Is there another way to circumvent that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I usually install in manually in a prior step.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where would you put it, do you have an example?

Copy link
Contributor

Choose a reason for hiding this comment

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

Here we install some system deps before everything else: https://github.com/oxidizing/sihl/blob/master/.github/workflows/ci.yml#L43

And then we run make deps which install a bunch of dev dependencies.

.PHONY: deps
deps:
	opam install -y dune-release merlin ocamlformat utop
	opam install -y alcotest-lwt mariadb.1.1.4 caqti-driver-postgresql.1.8.0 caqti-driver-mariadb.1.8.0
	opam install . -y --deps-only --locked
	eval $(opam env)

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the CI file, I think you can update to uses: avsm/setup-ocaml@v2 and then omit those anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

     - name: Use OCaml ${{ matrix.ocaml-version }}
        uses: ocaml/setup-ocaml@v2
        with:
          ocaml-compiler: ${{ matrix.ocaml-compiler }}
          dune-cache: true

Copy link
Contributor Author

@aronerben aronerben Aug 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This does not seem to work, I think we still need to install dependencies somewhere? It's saying ca-certs is missing now

run: |
opam install -y dune
opam install -y . --deps-only --with-doc --with-test --locked --unlock-base
opam install -y alcotest
- name: Recover from an Opam broken state
if: steps.cache-opam.outputs.cache-hit == 'true'
run: opam upgrade --fixup
Expand Down
5 changes: 2 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ build:
clean:
opam exec -- dune clean

.PHONY: test-all
test-all:
curl -s -d '{ "requestor": "letters", "version": "0.1.0" }' "https://api.nodemailer.com/user" -X POST -H "Content-Type: application/json" > ethereal_account.json
.PHONY: test
test:
opam exec -- dune runtest --force --no-buffer test
7 changes: 1 addition & 6 deletions test/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
(executable
(name test)
(libraries letters alcotest alcotest-lwt lwt))
(libraries letters alcotest))

(rule
(alias runtest)
(action
(run ./test.exe)))

(rule
(alias runtest-all)
(action
(run ./test.exe)))
44 changes: 20 additions & 24 deletions test/test.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ let stream_to_string s =
go ()
;;

let test_create_plain_text_email _ () =
let test_create_plain_text_email _ =
let recipients = [ To "[email protected]" ] in
let subject = "Hello" in
let body = Plain "Hello Dave" in
Expand All @@ -23,10 +23,10 @@ let test_create_plain_text_email _ () =
| Error reason -> failwith reason
in
let message = stream_to_string stream in
Lwt.return (print_string message)
print_string message
;;

let test_create_html_email _ () =
let test_create_html_email _ =
let recipients = [ To "[email protected]" ] in
let subject = "Hello" in
let body = Html "<i>Hello Dave</i>" in
Expand All @@ -37,10 +37,10 @@ let test_create_html_email _ () =
| Error reason -> failwith reason
in
let message = stream_to_string stream in
Lwt.return (print_string message)
print_string message
;;

let test_create_mixed_body_email _ () =
let test_create_mixed_body_email _ =
let recipients = [ To "[email protected]" ] in
let subject = "Hello" in
let body = Mixed ("Hello Dave", "<i>Hello Dave</i>", Some "blaablaa") in
Expand All @@ -51,26 +51,22 @@ let test_create_mixed_body_email _ () =
| Error reason -> failwith reason
in
let message = stream_to_string stream in
Lwt.return (print_string message)
print_string message
;;

let () =
Lwt_main.run
(Alcotest_lwt.run
"Email creation"
[ ( "Generating body"
, [ Alcotest_lwt.test_case
"email with plain text body"
`Quick
test_create_plain_text_email
; Alcotest_lwt.test_case
"email with HTML text body"
`Quick
test_create_html_email
; Alcotest_lwt.test_case
"email with mixed plain text and HTML body"
`Quick
test_create_mixed_body_email
] )
])
Alcotest.run
"Email creation"
[ ( "Generating body"
, [ Alcotest.test_case
"email with plain text body"
`Quick
test_create_plain_text_email
; Alcotest.test_case "email with HTML text body" `Quick test_create_html_email
; Alcotest.test_case
"email with mixed plain text and HTML body"
`Quick
test_create_mixed_body_email
] )
]
;;