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

Remove unix dependency in irmin-test #1860

Merged
merged 2 commits into from
Jul 8, 2022

Conversation

patricoferris
Copy link
Contributor

This PR removes the lwt.unix dependency in irmin-test allowing the test-suite package to be compiled into more exotic locations (the browser, unikernels etc.).

This is how we managed to test the irmin-server port to use websockets so the client could be in the browser. I then tested a few different backends (an updated irmin-indexeddb and irmin.mem) using the testsuite in this repository with a live version available.

Note this PR, in order to run in the browser, requires mirage/alcotest#348.

@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2022

Codecov Report

Merging #1860 (a6038c6) into main (347d885) will decrease coverage by 0.00%.
The diff coverage is 68.96%.

❗ Current head a6038c6 differs from pull request most recent head e897241. Consider uploading reports for the commit e897241 to get more accurate results

@@            Coverage Diff             @@
##             main    #1860      +/-   ##
==========================================
- Coverage   63.97%   63.96%   -0.01%     
==========================================
  Files         129      129              
  Lines       15479    15475       -4     
==========================================
- Hits         9902     9899       -3     
+ Misses       5577     5576       -1     
Impacted Files Coverage Δ
src/irmin-test/store_watch.ml 30.76% <50.00%> (+0.13%) ⬆️
src/irmin-test/common.ml 70.16% <61.90%> (-0.24%) ⬇️
bench/irmin-pack/trace_collection.ml 83.11% <100.00%> (-0.64%) ⬇️
src/irmin-test/store.ml 94.82% <100.00%> (+<0.01%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@icristescu icristescu added the no-changelog-needed No changelog is needed here label Jun 5, 2022
Copy link
Contributor

@icristescu icristescu left a comment

Choose a reason for hiding this comment

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

Thanks!

test/irmin-chunk/test.ml Outdated Show resolved Hide resolved
src/irmin-test/store.ml Outdated Show resolved Hide resolved
bench/irmin-pack/trace_collection.ml Show resolved Hide resolved
@samoht
Copy link
Member

samoht commented Jul 7, 2022

@patricoferris could you rebase your PR? That seems like a very good change, I'm happy to have it in the next irmin release :-)

@samoht
Copy link
Member

samoht commented Jul 7, 2022

(it also seems related to @metanivek's #1948)

@metanivek
Copy link
Member

@patricoferris my recent changes did touch similar files as this PR but looking at the conflicting files I think the only thing you need to know is that you can delete the test/irmin-unix directory. lmk if anything else comes up on rebase though. happy to help.

@patricoferris
Copy link
Contributor Author

@patricoferris could you rebase your PR? That seems like a very good change, I'm happy to have it in the next irmin release :-)

Great, all rebased :))

@samoht samoht merged commit 9bcce09 into mirage:main Jul 8, 2022
@samoht
Copy link
Member

samoht commented Jul 8, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog-needed No changelog is needed here
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants