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

Test do not run in IntelliJ #1065

Closed
abelsromero opened this issue Nov 10, 2021 · 11 comments
Closed

Test do not run in IntelliJ #1065

abelsromero opened this issue Nov 10, 2021 · 11 comments

Comments

@abelsromero
Copy link
Member

When trying to run a rest class or method directly from IntelliJ tests fail with

image

@abelsromero
Copy link
Member Author

Just mentioning it to let know I am looking into it because trying to work on anything else with this is just obnoxious.

So far I found that removing polutedTest task works, just need to find a permanent proper solution.

@abelsromero
Copy link
Member Author

Seems the issue comes from the categories filter in task pollutedTest

useJUnit {
    includeCategories 'org.asciidoctor.categories.Polluted'
  }

The command according to the run configuration is :asciidoctorj:test --tests "org.asciidoctor.jruby.cli.WhenAsciidoctorIsCalledUsingCli" which works fine from the terminal. But IDEA somehow messes things injecting --tests into the pollutedTest (which runs before test task) and that obviously fails because the test task does not match the category.
That explains why, running the tests with @Category(Polluted.class) works, it's run before tests.

@abelsromero
Copy link
Member Author

I reproduced the problem in a simple project an reported to Idea team https://youtrack.jetbrains.com/issue/IDEA-282425 🤞

@robertpanzer
Copy link
Member

Thanks for picking this up.
Sometimes it's really annoying. For me personally it seems to depend on the weather whether I can debug the tests or not.

@abelsromero
Copy link
Member Author

abelsromero commented Nov 12, 2021

As local workarround just removing test.dependsOn pollutedTest works, but that's not something we want in the long run.

EDIT: mistery solved, when the output already exists no error is shown and I was testing in 2 different paths.

@abelsromero
Copy link
Member Author

abelsromero commented Nov 14, 2021

I narrowed it down to missing dependencies introduced in asciidoctorj-diagram v2.2.0.

  implementation 'org.asciidoctor:asciidoctorj-diagram-ditaamini:1.0.1'
  implementation 'org.asciidoctor:asciidoctorj-diagram-plantuml:1.2021.8'

Adding them allows the distribution to convert the example successfully and explains why running from a Java app worked, since the dependencies where pulled automatically as transitives in the later. However, v2.5.2 of distribution contains asciidoctor-diagram 2.1.2 but I assume the fix from from plantuml itself-

Now @robertpanzer, as a fix I'd suggest

  1. Upgrade asciidoctorj-diagram to latest 2.2, just for maintenance.
  2. Remove the transitive=false restriction from which prevents pulling the new libs.
  3. Add some IT test to cover the reported issue. But I am not 100% on this because it feels something more on the territory of asciidoctorj-diagram, but without it we cannot avoid regressions on the CLI.

If you agree, I can create the PR tomorrow.

@robertpanzer
Copy link
Member

I just extracted the 2 modules recently for asciidoctorj-diagram 2.2.
True, I didn't upgrade asciidoctorj itself yet and the distribution.
I will take care of that. Integration tests should already be there.

@abelsromero
Copy link
Member Author

Is it necessary to handle them manually? I mean, why can't we just pull them as diagram's transitives?

@robertpanzer
Copy link
Member

In general asciidoctor-diagram-plantuml and asciidoctor-diagram-ditaamini have their own release cycle, that's why they also have their own version numbering scheme.
So to always have the latest and greatest components in the distribution (Ok, I missed that part now apparently :D) we could rely on the transitive dependency or we would need to specify their versions explicitly depending on the order in which the three modules were released.
I think it would be cleaner to just always reference them explicitly.

@abelsromero
Copy link
Member Author

I think it would be cleaner to just always reference them explicitly.

I see, I went back to the module descriptor and those are runtime decendencies, makes sense then 👍

@abelsromero
Copy link
Member Author

abelsromero commented Nov 21, 2021

My mistake, in the PR #1066, i mentioned this issue, but should have been #1064. This is still WIP.

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

2 participants