Skip to content

Add Scala.js handling of @Test annotations with arguments #7462

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
Dec 5, 2019

Conversation

Lacaranian
Copy link
Contributor

@Lacaranian Lacaranian commented Oct 28, 2019

To be able to use the expected and timeout annotation arguments in
JUnit @test annotations, add handling for these arguments by supplying
them as arguments to the resolved constructor for the JUnit test class.

Enable the JUnitAnnotationsParamTest, which depends on this
functionality.

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

  1. Separate subject from body with a blank line
  2. When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:
  3. Limit the subject line to 72 characters
  4. Capitalize the subject line
  5. Do not end the subject line with a period
  6. Use the imperative mood in the subject line ("Add" instead of "Added")
  7. Wrap the body at 80 characters
  8. Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

@Lacaranian
Copy link
Contributor Author

Notably, I'm not sure this one is quite there yet - I want to improve the pattern matching so as to match only the actual expected/timeout names, and further simplify it if possible.

@Lacaranian Lacaranian force-pushed the additional-sjs-tests branch from 54e49a2 to ea755e2 Compare October 28, 2019 01:21
@Lacaranian Lacaranian changed the title Partial fix for lampepfl#7113: Add Scala.js handling of @Test annotat… Add Scala.js handling of @Test annotations with arguments Oct 28, 2019
Partial fix for lampepfl#7113

To be able to use the `expected` and `timeout` annotation arguments in
JUnit @test annotations, add handling for these arguments by supplying
them as arguments to the resolved constructor for the JUnit test class.

Enable the JUnitAnnotationsParamTest, which depends on this
functionality.
@Lacaranian Lacaranian force-pushed the additional-sjs-tests branch from ea755e2 to 78e9a02 Compare October 29, 2019 03:22
@Lacaranian
Copy link
Contributor Author

The latest version of this is now in a reviewable state, though quite possibly still in need of some changes.

Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

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

Looks good to me for the Scala.js point of view.

Can someone from the dotty team check the coding style? I'm not sure about ifs in cases are supposed to be formatted in this codebase.

@smarter smarter merged commit 0a33ad3 into scala:master Dec 5, 2019
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.

4 participants