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

Support modern Flexmark #1885

Open
DanySK opened this issue Aug 31, 2020 · 7 comments
Open

Support modern Flexmark #1885

DanySK opened this issue Aug 31, 2020 · 7 comments

Comments

@DanySK
Copy link

DanySK commented Aug 31, 2020

Scalatest recently switched from pegdown to flexmark.
Besides a lack of documentation, as discussed in #1736, scalatest currently works with a small range of flexmark versions.

I propose to:

  1. add support for the more recent versions of Flexmark;
  2. possibly, declare Flexmark as a runtime dependency, so that users do not need to guess what's needed in order to make scalatest reporting system work;
  3. fail more clearly: the current error message is An exception or error caused a run to abort. This may have been caused by a problematic custom reporter.: it's truthful, but not particularly useful for resolution.

It is likely that the former two points make the third way less important, however my feeling is that the third is a low hanging fruit, while there might be need for more work to address the former.

@ldeck
Copy link

ldeck commented Feb 8, 2021

Ideally, this should be a plugin dependency so it doesn’t interfere with project dependencies. It should definitely be documented.

@caoilte
Copy link

caoilte commented Jun 23, 2021

Just ran into this issue trying to integrate with https://github.com/pact-foundation/pact-jvm on jdk8 (maybe modules makes it less of an issue on jdk9+, not sure) and which has a dependency on flexmark 0.62.2.

@DanySK where did you get to investigating an upgrade?

@DanySK
Copy link
Author

DanySK commented Jun 23, 2021

@caoilte I did not even start trying to upgrade scalatest, frankly. I just investigated where the issue was from. Worst case, I'll switch to kotest, Scala is becoming more and more marginal on most of my projects.

@cheeseng
Copy link
Contributor

Hi, fyi I submitted a PR to update flexmark version to 0.62.2:

#2050

@caoilte
Copy link

caoilte commented Jun 28, 2021

@cheeseng Hey nice! I was experimenting with pretty much the same changes using a locally built snapshot over the weekend. Weirdly, I found that even though the tests passed I couldn't use the snapshot (got runtime class not found) unless I added an extra dependency. Specifically,

"com.vladsch.flexmark" % "flexmark-profile-pegdown" % "0.62.2"

Would be interesting to know if you were also able to test locally?

@cheeseng
Copy link
Contributor

@caoilte I didn't try local snapshot, the tests should be using HtmlReporter to generate the html report under target/html. Afaik, to use local snapshot (even the real release build) we need to explicitly add the flexmark dependency like the following:

"com.vladsch.flexmark" % "flexmark-all" % "0.62.2"

The reason I think is the flexmark dependency is marked as "optional", here:

https://github.com/scalatest/scalatest/blob/3.2.x-new/project/scalatest.scala#L178

Your import does seems to be more specific than flexmark-all though, I wonder if scalatest should use flexmark-profile-pegdown instead.

@caoilte
Copy link

caoilte commented Jun 29, 2021

@cheeseng Just checked and you're right. It also works with flexmark-all and that makes sense because every project I know running scalatest 3.1+ has to manually add that dependency (ie #1736).

It would certainly be interesting to explore whether profile-pegdown is a better choice overall.

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

No branches or pull requests

4 participants