Skip to content

Break cyclical test dependency between html and css editor #8600

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

Closed
wants to merge 1 commit into from

Conversation

mbien
Copy link
Member

@mbien mbien commented Jun 18, 2025

second attempt to fix the failing jenkins build (first was #8584)

  • css.editor tests some features by embedding in html docs
  • html.editor has a direct dependency to css.editor
  • building css.edtor first would fail the build, e.g jenkins fails during clean ant build-nbm

I tried to move some mixed css/html tests to html.editor to remove the dependency entirely but test data and project tests are too tightly coupled atm. We could still do this but I didn't have time to pull this apart.

So lets make it worse and use reflection to downgrade the cyclical compiletime test dependency to runtime. Footprint is small at least.

cleaner solutions welcome, I suppose we could also not fix it and call build first on jenkins before building nbms (analog to what github actions do atm), or tweak the build-nbms ant target.

jenkins fails with:

[subant-junit] /mnt/ram/netbeans/ide/css.editor/test/unit/src/org/netbeans/modules/css/editor/csl/CssGotoDeclarationTest.java:23: error: package org.netbeans.modules.html.editor.gsf does not exist
[subant-junit] import org.netbeans.modules.html.editor.gsf.HtmlLanguage;
[subant-junit]                                            ^
[subant-junit] /mnt/ram/netbeans/ide/css.editor/test/unit/src/org/netbeans/modules/css/editor/csl/TestIssue166592.java:26: error: package org.netbeans.modules.html.editor.api.gsf does not exist
[subant-junit] import org.netbeans.modules.html.editor.api.gsf.HtmlParserResult;
[subant-junit]                                                ^
[subant-junit] /mnt/ram/netbeans/ide/css.editor/test/unit/src/org/netbeans/modules/css/editor/indent/CssIndenterTest.java:39: error: package org.netbeans.modules.html.editor.api does not exist
[subant-junit] import org.netbeans.modules.html.editor.api.HtmlKit;
[subant-junit]                                            ^
[subant-junit] /mnt/ram/netbeans/ide/css.editor/test/unit/src/org/netbeans/modules/css/editor/indent/CssIndenterTest.java:40: error: package org.netbeans.modules.html.editor.indent does not exist
[subant-junit] import org.netbeans.modules.html.editor.indent.HtmlIndentTaskFactory;
[subant-junit]                                               ^
[subant-junit] 4 errors

reproducible with:

  1. clone repo
  2. ant build-nbms

 - css.editor tests some features by embedding in html docs
 - html.editor has a direct dependency to css.editor
 - building css.edtor first would fail the build, e.g jenkins
   fails during clean 'ant build-nbm'

I tried to move some mixed css/html tests to html.editor to remove
the dependency entirely but test data and project tests are too tightly
coupled atm.

So lets make it worse and use reflection to downgrade the cyclical
compiletime test dependency to runtime. Footprint is small at least.
@mbien mbien added CSS [ci] enable web job HTML [ci] enable web job CI continuous integration changes tests labels Jun 18, 2025
@mbien mbien added this to the NB27 milestone Jun 18, 2025
@neilcsmith-net
Copy link
Member

As said in Slack conversation, I'd still like to understand why this broke recently. As far as I can tell, it started happening after the merge of #8401. It's certainly breaking at that point in the module graph. And the added test dependencies seem a little haphazard on which have recursive and compile-dependency. https://github.com/apache/netbeans/pull/8401/files#diff-716b69e0c3f302054894e97c98ff674cff32a12bd3984ce68a8fbd4ac7aaaedb

Not against this change if the cyclical test dependency is actually the problem in practice.

If build-nbms should require build then that change should go in the build files, not just be hidden by changing the Jenkins script. That might not be a bad change to make for consistency?

@mbien
Copy link
Member Author

mbien commented Jun 18, 2025

If build-nbms should require build then that change should go in the build files, not just be hidden by changing the Jenkins script. That might not be a bad change to make for consistency?

i wondered why build-nbms didn't do that, we can certainly fix it in ant too. Gh actions show that it works if it is built first. I am just a bit hesitant to change the release build tasks without checking that nothing unexpected changes.

@mbien mbien added build do not merge Don't merge this PR, it is not ready or just demonstration purposes. labels Jun 18, 2025
@mbien
Copy link
Member Author

mbien commented Jun 19, 2025

okidoki. I don't like reflection, so i close this in favor of #8604 which changed the dependency graph enough to avoid this issue and made jenkins pass again.

@mbien mbien closed this Jun 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build CI continuous integration changes CSS [ci] enable web job do not merge Don't merge this PR, it is not ready or just demonstration purposes. HTML [ci] enable web job tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants