Skip to content

Remove unix dependency in irmin-test#1860

Merged
samoht merged 2 commits into
mirage:mainfrom
patricoferris:no-unix-irmin-test
Jul 8, 2022
Merged

Remove unix dependency in irmin-test#1860
samoht merged 2 commits into
mirage:mainfrom
patricoferris:no-unix-irmin-test

Conversation

@patricoferris

Copy link
Copy Markdown
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

codecov-commenter commented Jun 5, 2022

Copy link
Copy Markdown

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

@icristescu icristescu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

Comment thread test/irmin-chunk/test.ml Outdated
Comment thread src/irmin-test/store.ml Outdated
Comment thread bench/irmin-pack/trace_collection.ml
@samoht

samoht commented Jul 7, 2022

Copy link
Copy Markdown
Member

@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

samoht commented Jul 7, 2022

Copy link
Copy Markdown
Member

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

@metanivek

Copy link
Copy Markdown
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 and others added 2 commits July 7, 2022 18:48
Co-authored-by: Ioana Cristescu  <ioana@tarides.com>
@patricoferris

Copy link
Copy Markdown
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

samoht commented Jul 8, 2022

Copy link
Copy Markdown
Member

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