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

Add Scala.js support #166

Merged
merged 4 commits into from
Sep 24, 2020
Merged

Add Scala.js support #166

merged 4 commits into from
Sep 24, 2020

Conversation

sbrunk
Copy link
Contributor

@sbrunk sbrunk commented Sep 14, 2020

This is an attempt to add Scala.js support to scalacheck-toolbox so that downstream projects can use it for tests running on Scala.js.

It adds Scala.js support to the datetime combinators modules. I left support for magic out for now as it would require changes in how blns.txt gets loaded.

Note that I've added sbt-projectmatrix for sane cross-building but I had to remove sbt-modules along the way.

Fixes #25

@sbrunk
Copy link
Contributor Author

sbrunk commented Sep 18, 2020

It doesn't look like the check failure is related to my changes, but I'm not familiar enough with the project to know for sure.

Copy link
Contributor

@BenFradet BenFradet left a comment

Choose a reason for hiding this comment

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

LGTM 👍. Seems like the mdoc plugin now makes warnings fail the build. I'll try to fix it.

build.sbt Outdated Show resolved Hide resolved
@BenFradet
Copy link
Contributor

can you rebase against master, ci should be fixed

@BenFradet
Copy link
Contributor

the warning which makes the build fails is related to scala 2.13.3

@codecov
Copy link

codecov bot commented Sep 22, 2020

Codecov Report

Merging #166 into master will increase coverage by 0.71%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
+ Coverage   95.68%   96.40%   +0.71%     
==========================================
  Files          10       10              
  Lines         139      139              
  Branches        2        2              
==========================================
+ Hits          133      134       +1     
+ Misses          6        5       -1     
Impacted Files Coverage Δ
...ysevendeg/scalacheck/datetime/instances/joda.scala 100.00% <ø> (ø)
...rtysevendeg/scalacheck/datetime/joda/GenJoda.scala 94.44% <ø> (ø)
...scalacheck/datetime/joda/granularity/package.scala 100.00% <ø> (ø)
...ala/com/fortysevendeg/scalacheck/magic/Magic.scala 100.00% <ø> (ø)
...ortysevendeg/scalacheck/datetime/GenDateTime.scala 100.00% <0.00%> (+20.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9dc3e81...604ad99. Read the comment docs.

BenFradet
BenFradet previously approved these changes Sep 22, 2020
Copy link
Contributor

@BenFradet BenFradet left a comment

Choose a reason for hiding this comment

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

@sbrunk
Copy link
Contributor Author

sbrunk commented Sep 22, 2020

Running the tests now takes almost 2 hours compared to ~18 minutes before 🙀

The Scala.js tests run on node.js. Seems like they are by a magnitude slower especially on constrained CI runners.

@sbrunk
Copy link
Contributor Author

sbrunk commented Sep 23, 2020

Ok it seems like only genPickFromMap* in CombinatorProperties is crazy slow on JS. Would you mind if I reduce the number of required successful tests from 100000 to 10000 (only when running on Scala.js of course)?

override def overrideParameters(p: Test.Parameters): Test.Parameters =
p.withMinSuccessfulTests(100000)

@BenFradet
Copy link
Contributor

fine with me as long as it's only for js 👍

Copy link
Contributor

@BenFradet BenFradet left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@fedefernandez fedefernandez left a comment

Choose a reason for hiding this comment

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

LGTM!

@BenFradet BenFradet merged commit 7eb421e into 47degrees:master Sep 24, 2020
@sbrunk sbrunk deleted the scala-js-support branch September 25, 2020 08:25
@sbrunk
Copy link
Contributor Author

sbrunk commented Oct 19, 2020

Dear maintainers,
do you have plans to cut a release in the near future? I'd love to use this library in a downstream project but I depend on a published version with Scala.js support. Thanks a lot!

@BenFradet
Copy link
Contributor

yes, we should be releasing 0.4.0 today 👍

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.

Build for Scala JS
3 participants