[DOXIA-616] #34

bertysentry · 2021-01-03T19:26:10Z

Fixed integration tests for *doxia-formats wrt fenced code blocks
Upgraded to Doxia 1.10-SNAPSHOT

* Fixed integration tests for **doxia-formats* wrt fenced code blocks * Upgraded to Doxia 1.10-SNAPSHOT

bertysentry · 2021-01-03T19:29:21Z

@michael-o Please note:

I upgraded to Doxia 1.10-SNAPSHOT (not sure you wanted this here, just let me know)
I wondered whether we should actually remove these Doxia markdown integration tests from maven-site-plugin, as we now have proper integration tests in doxia-module-markdown itself. This would prevent such situations where updates in one project (Doxia) requires to update the tests in another project (maven-site-plugin).

Please advise! :-)

michael-o · 2021-01-03T20:39:48Z

Thanks, will look into it.

elharo

Can this change wait until doxia 1.10 is released?

michael-o · 2021-01-04T00:15:49Z

I won't merge before anyway because I never depend on snapshots.

bertysentry · 2021-01-04T18:35:53Z

Okay, I will update the PR once Doxia 1.10 is released.

Do you guys have any opinion on the removal of markdown-specific integration tests (especially Doxia-specific) from maven-site-plugin, and reincorporate them in Doxia? This would avoid the chicken-egg situation like this.

michael-o · 2021-01-04T22:58:26Z

In my opinion they need to bemoved because we test the entire site generation, and not Doxia specific modules. @hboutemy WDYT?

elharo · 2021-01-05T11:51:39Z

I think I agree that these should be moved. If they're only testing doxia functionality, and we can test them in doxia, then that's better. Or perhaps tests belong in both places. Doxia should be an implementation detail of maven-site-plugin. Any tests we have for HTML generation in sites should be independent of the HTML generator we're using.

hboutemy · 2021-01-05T22:18:26Z

is it causing issues, so they need to be removed?
if you're sure you have sufficient tests nowadays in Doxia, Doxia Sitetools and skins to check that a new feature in a Doxia module has the expected impact when used in a real site, you can drop this end to end test
but I feel that a few end to end tests remain useful to check that the full rendering bring expected result after execution of https://maven.apache.org/plugins/maven-site-plugin/history.html

bertysentry · 2021-01-05T22:37:19Z

@hboutemy We will keep some integration tests in maven-site-plugin that involve doxia-module-markdown. I want to remove specific Doxia tests (like "quotes were mistakenly removed", or "code block must be surrounded with <div class="source">.

These can be properly tested in Doxia with unit tests, and also with integration tests. Where applicable, we use integration tests to invoke maven-site-plugin with the current version of Doxia and make sure the end-to-end chain works as expected.

I will submit a PR on Doxia to incorporate these specific tests (from maven-site-plugin) to doxia-module-markdown.

Watch for DOXIA-618

Edit: added reference to JIRA issue number

bertysentry · 2021-01-05T23:47:31Z

Submitted PR for Doxia: apache/maven-doxia#52

Thank you!

* Removed Doxia-specific integration tests * Added integration tests on the various document formats * Added integration tests on macros processing

* Fixed broken integration tests on **doxia-formats** on Linux and MacOS * Integration tests on macros are done on the SNIPPET macro from a common Java source file

bertysentry · 2021-01-13T23:06:34Z

Okay, PR has been updated as below:

Removed Doxia-specific integration tests
Added integration tests on the various document formats
Integration tests on macros are done on the SNIPPET macro from a common Java source file

Please note that the builds on macOS with JDK11, 14 and 15 fail, but these failures are not related to this PR (master is failing the same way).

@hboutemy @elharo @michael-o Please review these changes. Thank you!

* Reverted Doxia version to 1.9.1

bertysentry · 2021-01-14T11:02:38Z

Oh, last update: I reverted Doxia version to 1.9.1. MSITE integration tests will pass with both 1.9.1 and 1.10 (no more checks on specific Doxia formatting features).

bertysentry · 2021-01-14T11:28:39Z

More build failures: Connection timeout with www.w3.org, which is totally unrelated to this PR.

Edit: Things run smoothly on my fork. See https://github.com/bertysentry/maven-site-plugin/runs/1701505614?check_suite_focus=true

elharo · 2021-01-14T12:17:07Z

We really should resolve the CI issues before proceeding with this or any other changes.

bertysentry · 2021-01-14T13:01:43Z

@elharo IMO there is no need to wait to merge this PR. This PR ensures the integration tests won't break when upgrading to Doxia 1.10.

The integration tests that currently fail are only for macOS width JDK 11, 14 and 15:

Error: MSITE-665/pom.xml: See MSITE-665 from 2013
Error: MSITE-512/pom.xml
Error: MSITE-506/pom.xml
Error: MSITE-497/pom.xml
Error: MSITE-484/pom.xml

They are all ancient issues and completely unrelated to the changes in the integration tests to support Doxia 1.10.

Thanks.

bertysentry · 2021-01-14T18:21:34Z

@elharo Any idea on how to retrieve the build.log files from GitHub Actions/Workflows? I must be missing something here, but I can't find any link to any downloadable artifact in GitHub Actions... 😅

bertysentry · 2021-01-18T11:28:36Z

@hboutemy @michael-o @elharo Will you have a chance to review these changes? This ensures the maven-site-plugin integration tests don't break when upgrading from Doxia 1.9.1 to 1.10.

elharo · 2021-01-18T11:47:06Z

Before this can move further, someone needs to debug and fix the CI failures. That's likely not caused by this PR, but checking in when the CI is broken is never a good idea.

bertysentry · 2021-01-18T11:48:56Z

@elharo We cannot access the integration test logs, how are we going to fix this?

elharo · 2021-01-18T12:02:23Z

It might need someone who has more permissions on the project. I'm not sure.

bertysentry · 2021-01-18T12:05:08Z

I have full permissions on my fork, and it's running all GitHub Actions as well, and failing at the same place (macOS with JDK11, 14 and 15). But I can't find a way to access the files produced by the build (not even the plugin artifact).

elharo · 2021-01-18T12:06:26Z

Some though not all of the errors seem related to this:

: | [ERROR] Failed to execute goal org.apache.maven.plugins:maven-site-plugin:3.10.0-SNAPSHOT:site (default-cli) on project test: Error parsing '/home/runner/work/maven-site-plugin/maven-site-plugin/target/it/validate/src/site/fml/faq.fml': line [-1] Error validating the model: Connect to www.w3.org:80 [www.w3.org/128.30.52.100] failed: Connection timed out -> [Help 1]

It might be worth understanding why the test is trying to connect to www.3.org. I'm not sure it needs to do that.

bertysentry · 2021-01-18T12:08:00Z

This was just a temporary network issue of GitHub Actions. If you re-run the workflow, you won't get these errors.

bertysentry · 2021-01-18T12:11:18Z

Also, PR #35 just got merged to master 3 days ago, with the same CI failures.

elharo · 2021-01-18T19:19:56Z

That one wasn't merged by me. :-)

bertysentry · 2021-01-18T22:19:02Z

One recurring error is:

Error: |        [ERROR] Failed to execute goal org.apache.maven.plugins:maven-site-plugin:3.10.0-SNAPSHOT:site (default-cli) on project test: Error parsing '/home/runner/work/maven-site-plugin/maven-site-plugin/target/it/validate/src/site/fml/faq.fml': line [-1] Error validating the model: Connect to www.w3.org:80 [www.w3.org/128.30.52.100] failed: Connection timed out (Connection timed out) -> [Help 1]

It happens when the site:validate goal is invoked in the integration tests (in src/it/validate/pom.xml). Apparently, site:validate tries to download the specified XML schema to validate the source documents (FML and XDOC). On GitHub Actions, this appears to fail from time to time.

This is nothing too serious, IMO.

bertysentry · 2021-01-24T19:30:42Z

@michael-o @hboutemy @elharo Friendly reminder that this PR is still waiting, and its content is not going to change with regards to the failed builds, which are caused by something else (master is failing the same builds the same way).

The more we wait, the more risk we have to get conflicting changes.

Thanks!

michael-o · 2021-01-24T21:19:49Z

I won't be able to review before mid Feb due to circumstances here.

slachiewicz · 2021-01-24T22:11:50Z

I'll look at it tomorrow

bertysentry · 2021-01-27T18:57:04Z

Guys? I'm still waiting for this to be merged so I can move forward with more changes in Doxia. Your help is much appreciated 😉

michael-o · 2021-01-27T19:45:36Z

Guys? I'm still waiting for this to be merged so I can move forward with more changes in Doxia. Your help is much appreciated 😉

As promised, I will pick up as soon as I can, but now Oracle quartely updates are waiting for me...

* Fixed integration tests for **doxia-formats* wrt fenced code blocks * Removed Doxia-specific integration tests * Added integration tests on the various document formats * Added integration tests on macros processing Closes #34

slachiewicz

I squashed commits - looks good to me.
Only MacOS tests failed https://github.com/apache/maven-site-plugin/actions/runs/516170331
Our Jenkins is green https://ci-builds.apache.org/job/Maven/job/maven-box/job/maven-site-plugin/job/feature%252FDOXIA-616/

michael-o · 2021-01-30T12:41:03Z

Let me go through too...

michael-o · 2021-01-30T13:54:07Z

I squashed commits - looks good to me.
Only MacOS tests failed https://github.com/apache/maven-site-plugin/actions/runs/516170331
Our Jenkins is green https://ci-builds.apache.org/job/Maven/job/maven-box/job/maven-site-plugin/job/feature%252FDOXIA-616/

I was not able to reproduce this problem multiple times.

bertysentry · 2021-01-30T17:12:02Z

Awesome, thank you guys! 😊

[DOXIA-616]

44dec2c

* Fixed integration tests for **doxia-formats* wrt fenced code blocks * Upgraded to Doxia 1.10-SNAPSHOT

elharo reviewed Jan 3, 2021

View reviewed changes

bertysentry added 2 commits January 13, 2021 21:53

[DOXIA-616]

3b22895

* Removed Doxia-specific integration tests * Added integration tests on the various document formats * Added integration tests on macros processing

[DOXIA-616]

32d55a6

* Fixed broken integration tests on **doxia-formats** on Linux and MacOS * Integration tests on macros are done on the SNIPPET macro from a common Java source file

[DOXIA-616]

89cc3b2

* Reverted Doxia version to 1.9.1

slachiewicz self-requested a review January 27, 2021 20:56

slachiewicz approved these changes Jan 27, 2021

View reviewed changes

asfgit closed this in baba99a Jan 30, 2021

bertysentry deleted the feature/DOXIA-616-fix-site-integration-tests branch January 30, 2021 17:12

[DOXIA-616] #34

[DOXIA-616] #34

Conversation

bertysentry commented Jan 3, 2021

bertysentry commented Jan 3, 2021

michael-o commented Jan 3, 2021

elharo left a comment

Choose a reason for hiding this comment

michael-o commented Jan 4, 2021

bertysentry commented Jan 4, 2021

michael-o commented Jan 4, 2021

elharo commented Jan 5, 2021

hboutemy commented Jan 5, 2021

bertysentry commented Jan 5, 2021 • edited

bertysentry commented Jan 5, 2021

bertysentry commented Jan 13, 2021

bertysentry commented Jan 14, 2021

bertysentry commented Jan 14, 2021 • edited

elharo commented Jan 14, 2021

bertysentry commented Jan 14, 2021

bertysentry commented Jan 14, 2021

bertysentry commented Jan 18, 2021

elharo commented Jan 18, 2021

bertysentry commented Jan 18, 2021

elharo commented Jan 18, 2021

bertysentry commented Jan 18, 2021

elharo commented Jan 18, 2021

bertysentry commented Jan 18, 2021

bertysentry commented Jan 18, 2021

elharo commented Jan 18, 2021

bertysentry commented Jan 18, 2021

bertysentry commented Jan 24, 2021

michael-o commented Jan 24, 2021

slachiewicz commented Jan 24, 2021

bertysentry commented Jan 27, 2021

michael-o commented Jan 27, 2021

slachiewicz left a comment • edited

Choose a reason for hiding this comment

michael-o commented Jan 30, 2021

michael-o commented Jan 30, 2021

bertysentry commented Jan 30, 2021

bertysentry commented Jan 5, 2021 •

edited

bertysentry commented Jan 14, 2021 •

edited

slachiewicz left a comment •

edited