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

Add JUnit config to intellijinit #133

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pejovica
Copy link
Member

Makes intellijinit generate default run/debug configuration for JUnit.

There are two aspects to proposed changes. First one is the placement of generated configuration. At the moment it is stored in the workspace.xml, since the official location is not available yet (IDEA-65915). And the second one is the collection of necessary VM parameters for JUnit to run. I tried to reuse mx unittest for that by effectively making it execute a dry run. This implementation of dry run only prevents tests from running. It doesn't prevent side effects, e.g., unittest's test discovery and caching. But since unittest allows custom launchers, i.e., implemented by other modules, I am not sure what can be expected from them in this regard. Graal core seems to work fine.

@vjovanov vjovanov self-requested a review April 26, 2017 14:09
@vjovanov
Copy link
Member

@pejovica could you please sign the oracle contributor agreement? Here are the instructions:
http://www.oracle.com/technetwork/community/oca-486395.html
Also, the gate is failing due to style issues. To reproduce locally run:

mx --strict-compliance gate --strict-mode

@vjovanov
Copy link
Member

Otherwise, LGTM

 - Make IntelliJ builds fail on Ant Exec task error

 - Pass IntelliJ project's JDK to Ant build tasks, so that they are
   independent of the environment (i.e., they run even without
   JAVA_HOME being set)

 - Make ideclean remove IntelliJ's modules that were created by
   running intellijinit with --mx-python-modules flag
@pejovica
Copy link
Member Author

I've added the final set of changes, so could you have another look?

mx.py Outdated
@@ -13166,6 +13166,85 @@ def artifactFileName(dist):
artifactFile = join(artifactsDir, artifactFileName(dist))
update_file(artifactFile, artifactXML.xml(indent=' ', newl='\n'))

# JUnit configuration
def get_unittest_args():
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit invasive: it introduces an invisible dependency between this code and unittest and these eventually fire back. We need to re-organize unittest such that it uses the same function this code will use. This function will simply return all the arguments.

Also, in this way, there will be less code.

mx.py Outdated
default_configuration = ''.join([line.strip() for line in default_configuration.splitlines()])

# until the official solution becomes available (IDEA-65915), use `workspase.xml`
wsXml = XMLDoc()
Copy link
Member

Choose a reason for hiding this comment

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

We can't just nuke someones workspace someone might of spent hours on setting it up. We need to find <configuration default="true" type="JUnit" factoryName="JUnit"> and replace it with the new one.

mx.py Outdated
'''.format(vm_params=get_unittest_args())

# remove all whitespace between tags, so that we don't end up with excess newlines in xml output
default_configuration = ''.join([line.strip() for line in default_configuration.splitlines()])
Copy link
Member

Choose a reason for hiding this comment

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

This is not good as it breaks indentation. Try some of these that you find the easiest:

http://stackoverflow.com/questions/2504411/proper-indentation-for-python-multiline-strings

Sad that python does not support this. You can also do what Scala does just with a regex here.

mx.py Outdated
# remove all whitespace between tags, so that we don't end up with excess newlines in xml output
default_configuration = ''.join([line.strip() for line in default_configuration.splitlines()])

# until the official solution becomes available (IDEA-65915), use `workspase.xml`
Copy link
Member

Choose a reason for hiding this comment

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

workspase ~> workspace

zapster added a commit to zapster/mx that referenced this pull request Jul 1, 2017
…mx:mx-benchmark-jmh to master

* commit '34ed8b50032600d6f2d3aac4584470bcef3df6b0':
  work around pylint bug
  Bump version to 5.32.0
  mx benchmark: add a default vm and jmh benchmark (if mx is primary suite)
  mx benchmark: add support for JMH
@graalvmbot
Copy link
Collaborator

  • It appears that user Aleksandar Pejović with email address pejovica -(at)- users -(dot)- noreply -(dot)- github -(dot)- com has not signed the Oracle Contributor Agreement.

If you believe this is an error, please contact [email protected].

@graalvmbot
Copy link
Collaborator

  • Hello Aleksandar Pejović, thanks for contributing a PR to our project!

We use the Oracle Contributor Agreement to make the copyright of contributions clear. We don't have a record of you having signed this yet, based on your email address pejovica -(at)- users -(dot)- noreply -(dot)- github -(dot)- com. You can sign it at that link.

If you think you've already signed it, please comment below and we'll check.

@vjovanov
Copy link
Member

@pejovica will there be progress on this or we can close the PR?

@graalvmbot
Copy link
Collaborator

Hello Aleksandar Pejović, thanks for contributing a PR to our project!

We use the Oracle Contributor Agreement to make the copyright of contributions clear. We don't have a record of you having signed this yet, based on your email address pejovica -(at)- users -(dot)- noreply -(dot)- github -(dot)- com. You can sign it at that link.

If you think you've already signed it, please comment below and we'll check.

@vjovanov
Copy link
Member

@pejovica is this still valid?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants