Skip to content

Execute tests/run/ tests with Scala.js. #15543

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

Merged
merged 1 commit into from
Jul 4, 2022

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented Jun 28, 2022

We add support for skipping tests based on "toolArgs" flags, written in the test sources as

// scalajs: --skip

The ScalaJSCompilationTests takes the scalajs toolArgs into account, while the JVM CompilationTests do not. This effectively means that tests marked as above are run only on the JVM.

Good reasons for being skipped on Scala.js include:

  • Using .java source files.
  • Using Java reflection (sometimes to implement the testing logic, more than what gets tested).
  • Relying on NullPointerExceptions being thrown.
  • Testing the precise result of getClass() of classOf, which is not the same for BoxedUnit in Scala.js (these cases are extensively tested in Scala.js' own test suite, so they are well covered).

Some tests are skipped for less good reasons, for example:

  • The Course-* tests are long, ancient tests that don't test anything in particular, but rely a lot on the precise printed form of Doubles.

Some tests rely on what Scala.js calls "compliant semantics". They receive the flag --compliant-semantics to instruct the Scala.js linker to enable them.

Many tests, due to the println-based nature of Vulpix, rely on the precise printed form of Doubles and Units. When reasonably practical, these have been amended to pass one way or another on Scala.js:

  • using non-whole Doubles, or
  • explicitly trimming trailing .0 from the string representation of doubles, or
  • explicitly converting () to "()", or
  • using other data types, or
  • printing Doubles instead of Floats, or
  • avoiding to print irrelevant details.

Tests that are supposed to pass, but do not because of what seems to be a bug, are marked

// scalajs: --skip --pending

so that we can grep for them later and try to fix them.

@sjrd sjrd force-pushed the sjs-vulpix-run-tests branch 2 times, most recently from 1978640 to 4a8d1fa Compare June 30, 2022 07:58
We add support for skipping tests based on "toolArgs" flags,
written in the test sources as

  // scalajs: --skip

The `ScalaJSCompilationTests` takes the `scalajs` toolArgs
into account, while the JVM `CompilationTests` do not. This
effectively means that tests marked as above are run only
on the JVM.

Good reasons for being skipped on Scala.js include:

* Using `.java` source files.
* Using Java reflection (sometimes to implement the testing
  logic, more than what gets tested).
* Relying on `NullPointerException`s being thrown.
* Testing the precise result of `getClass()` of `classOf`,
  which is not the same for `BoxedUnit` in Scala.js (these
  cases are extensively tested in Scala.js' own test suite,
  so they are well covered).

Some tests are skipped for less good reasons, for example:

* The `Course-*` tests are long, ancient tests that don't
  test anything in particular, but rely a lot on the
  precise printed form of Doubles.

Some tests rely on what Scala.js calls "compliant semantics".
They receive the flag `--compliant-semantics` to instruct the
Scala.js linker to enable them.

Many tests, due to the `println`-based nature of Vulpix,
rely on the precise printed form of `Double`s and `Unit`s.
When reasonably practical, these have been amended to pass
one way or another on Scala.js:

* using non-whole `Double`s, or
* explicitly trimming trailing `.0` from the string
  representation of doubles, or
* explicitly converting `()` to `"()"`, or
* using other data types, or
* printing `Double`s instead of `Float`s, or
* avoiding to print irrelevant details.

Tests that are supposed to pass, but do not because of what
seems to be a bug, are marked

  // scalajs: --skip --pending

so that we can grep for them later and try to fix them.
@sjrd sjrd force-pushed the sjs-vulpix-run-tests branch from 4a8d1fa to 3518226 Compare June 30, 2022 08:57
@sjrd sjrd marked this pull request as ready for review June 30, 2022 08:58
@sjrd sjrd requested a review from dwijnand June 30, 2022 08:58
@sjrd
Copy link
Member Author

sjrd commented Jun 30, 2022

@dwijnand May I give you this one?

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

LGTM. cc @som-snytt too

@sjrd
Copy link
Member Author

sjrd commented Jul 4, 2022

Should we merge this before it grows stale? Any objection?

@dwijnand dwijnand merged commit 7d6d42f into scala:main Jul 4, 2022
@dwijnand dwijnand deleted the sjs-vulpix-run-tests branch July 4, 2022 09:33
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.

2 participants