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

Migrate to AsciiDoc snippets (#595 / #632) #632

Merged
merged 19 commits into from
May 19, 2022

Conversation

Bukama
Copy link
Member

@Bukama Bukama commented May 13, 2022

Proposed commit message:

Migrate to AsciiDoc snippets (#595 / #632)

Migrate all demo code to AciiDoc snippets to let them be compiled.
Intentionally failing tests are moved to inner classes to prevent
test execution.

closes #595 
PR: #632

@@ -260,6 +260,7 @@ tasks {
dependencies {
implementation(project)
implementation("com.fasterxml.jackson.core:jackson-databind:$jacksonVersion")
implementation("org.assertj:assertj-core:3.22.0")
Copy link
Member Author

Choose a reason for hiding this comment

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

This seems not to work as assertThat is not found during compile (but by IntelliJ) in demo sources

@Bukama
Copy link
Member Author

Bukama commented May 13, 2022

Aside the fact that assertThat is found by IntellJ but not by Gradle there are two open questions in the issue about the TempDir and disable-if-display-name.

@nipafx
Copy link
Member

nipafx commented May 14, 2022

Fixed it. Now the build fails because some of the demos fail at run time. Intentionally, I think, but that means those failing demos must be pulled into nested classes that aren't marked as @Nested and hence not executed by Jupiter. To be super-thorough, you could run them indirectly (like our tests do) and then assert the correct behavior. You could probably copy-paste that from the actual tests.

@Bukama
Copy link
Member Author

Bukama commented May 15, 2022

Fixed it. Now the build fails because some of the demos fail at run time. Intentionally, I think, but that means those failing demos must be pulled into nested classes that aren't marked as @Nested and hence not executed by Jupiter. To be super-thorough, you could run them indirectly (like our tests do) and then assert the correct behavior. You could probably copy-paste that from the actual tests.

Thank you. I moved the failing tests to inner classes and the build successfully runs now (at least local).

One thing I noticed: As for the demo the same spotless/checkstyle rules are applied the code is intended. These intention is then also applied to the documentation. Same for all other formatting, which results in longer code lines than we had before after applying #317.

@Bukama Bukama marked this pull request as ready for review May 15, 2022 17:52
@Bukama Bukama requested review from nipafx, Michael1993 and beatngu13 and removed request for nipafx and Michael1993 May 15, 2022 17:52
@Bukama Bukama changed the title Bishue595/asciidocsnippets Migrate to AsciiDoc snippets (#595 / #632) May 15, 2022
@nipafx
Copy link
Member

nipafx commented May 16, 2022

You need to add indent=0 to the include's option list, e.g.:

include::{json-demo}[tag=classpath_source,indent=0]

@nipafx nipafx merged commit e4fed6d into junit-pioneer:main May 19, 2022
@nipafx
Copy link
Member

nipafx commented May 19, 2022

Thank you very much, Bish! ❤️

nipafx pushed a commit that referenced this pull request May 19, 2022
Migrate all demo code to AciiDoc snippets to let them be compiled.
Intentionally failing tests are moved to inner classes to prevent
test execution.

Closes: #595
PR: #632
@Bukama Bukama deleted the bishue595/asciidocsnippets branch May 22, 2022 16:17
Bukama added a commit to Bukama/junit-pioneer that referenced this pull request Sep 20, 2022
Migrate all demo code to AciiDoc snippets to let them be compiled.
Intentionally failing tests are moved to inner classes to prevent
test execution.

Closes: junit-pioneer#595
PR: junit-pioneer#632
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.

None yet

2 participants