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] Remove duplicated Dummy language module #2435

Closed
Tracked by #3898
oowekyala opened this issue Apr 23, 2020 · 4 comments · Fixed by #4399
Closed
Tracked by #3898

[test] Remove duplicated Dummy language module #2435

oowekyala opened this issue Apr 23, 2020 · 4 comments · Fixed by #4399
Labels
an:enhancement An improvement on existing features / rules in:testing About tests of pmd, eg the module pmd-lang-test or pmd-test is:feature-removal Remove an unneeded unused feature
Milestone

Comments

@oowekyala
Copy link
Member

Is your feature request related to a problem? Please describe.
There's currently a "dummy" language module in the pmd-core test sources and one in the main pmd-test sources. They're a pain to keep in sync on the 7.0.x branch, and they're perfect duplicates. It also seems like the only usages of those DummyNodes in the pmd-test module are... in the tests of pmd-test

Describe the solution you'd like
Just make the test sources of pmd-test depend on the test sources of pmd-core, and remove the dummy module from pmd-test. https://stackoverflow.com/questions/174560/sharing-test-code-in-maven

This would remove the ability to use dummy nodes in external code that depends on pmd-test, but I think, they may use a real language module to fake nodes? I can't really think of a reason why anyone would want to build fake trees by hand outside of our test sources

Describe alternatives you've considered

  1. Don't change anything, keep syncing the sources manually. This is a pain
  2. Put the "dummy" language module in the main sources of PMD core. This would have some advantages, but then we'd have to make a commitment to binary compatibility over versions... and it would show up in LanguageRegistry
@oowekyala oowekyala added the an:enhancement An improvement on existing features / rules label Apr 23, 2020
@oowekyala oowekyala added this to the 7.0.0 milestone Apr 23, 2020
@adangel
Copy link
Member

adangel commented Apr 24, 2020

I didn't look into the details yet, but

It also seems like the only usages of those DummyNodes in the pmd-test module are... in the tests of pmd-test

would it be possible to move these tests then to pmd-core?

@oowekyala
Copy link
Member Author

I looked a bit into it, and apparently there are only two unit tests in the module. But afaik they can't be moved to pmd-core, since they need the RuleTst class

@oowekyala
Copy link
Member Author

I now think we should move PlainTextLanguage (pmd/pmd-designer#25) from the designer into the main sources of PMD core. The goal is not to write rules for plain text files, so we don't have to register this language in LanguageRegistry. We could just make it a singleton available with PlainTextLanguage.getInstance().

The advantages of this would be

  • There is always a language to fall back on if you need one (which is why the designer has that PlainTextLanguage)
  • TextDocuments/ TextFile ([core] Text documents epic #3784) always require a language to be specified. It is convenient to have a neutral language when the only thing you want to do with a TextDocument is to query line/columns and stuff, and don't plan to parse it or execute rules. Use cases for this have popped up in the designer (Compat with pmd/pmd#3893 pmd-designer#51)
  • Tests of pmd-test and pmd-test-schema ([test] Extract xml schema module #3976) would use this instead of a DummyLanguageModule, they can just use plain text. DummyLanguageModule is useful to mock nodes and set properties on them, but only the tests of pmd-core currently use this flexibility.

@oowekyala
Copy link
Member Author

Another possibility is to make the test classes (and resources) of pmd-core visible to the tests of other modules. This is what #4089 does for pmd-ant, to get access to dummy languages, rules and rulesets. This is maybe the easiest way.

@adangel adangel added in:testing About tests of pmd, eg the module pmd-lang-test or pmd-test is:feature-removal Remove an unneeded unused feature labels Jan 13, 2023
@adangel adangel mentioned this issue Jan 23, 2023
55 tasks
@oowekyala oowekyala linked a pull request Feb 18, 2023 that will close this issue
4 tasks
adangel added a commit to adangel/pmd that referenced this issue Mar 2, 2023
adangel added a commit to adangel/pmd that referenced this issue Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
an:enhancement An improvement on existing features / rules in:testing About tests of pmd, eg the module pmd-lang-test or pmd-test is:feature-removal Remove an unneeded unused feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants