Page MenuHomePhabricator

Add arc unit engine for gradle
ClosedPublic

Authored by Hackintosh5 on Feb 4 2019, 8:12 PM.

Details

Summary

Add home-grown non-GM Gradle unit-tester, resolves T315

Test Plan

Run arc unit and see if it gives correct messages when test pass, fail.

Diff Detail

Repository
rLIBWFJVA WildFyre Java Libary
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Info-Screen added inline comments.
.arclint
29

This linter should be the only one removed

This revision now requires changes to proceed.Feb 4 2019, 8:17 PM
This revision is now accepted and ready to land.Feb 4 2019, 8:39 PM
arcgradle/unit/GradleUnitTestEngine.php
44

You have SpotBugs twice, is that intentional?

This revision now requires review to proceed.Feb 5 2019, 6:36 AM

I need to test this locally before accepting, just to be sure. Sorry, it might take a few days before I have the time.

arcgradle/unit/GradleUnitTestEngine.php
44

I have unit tests twice too, once for pass, once for fail. Really I should do only the setResult in the if clause but it's simpler to do the whole thing

Push accidental code, now removed

  • Autostart the development API if it's not running already, and stop it again when we're done.
  • Remove debug and unused code
  • Autostart the development API if it's not running already, and stop it again when we're done.

That's really not a good idea, the API takes 2-4 minutes to launch. Just abort if it doesn't run, that'll be much more convenient.

arcgradle/__phutil_library_map__.php
7

If this is generated code, maybe it shouldn't be added to the repo at all? Correct me if I'm wrong I don't know how Arc plugins work

Hackintosh5 added inline comments.
arcgradle/__phutil_library_map__.php
7

Nope if its not there the plugin won't load. If the generated code wasn't needed, why generate it in the first place?

arcgradle/__phutil_library_map__.php
7

How and when is it generated?

In D218#5234, @CLOVIS wrote:
  • Autostart the development API if it's not running already, and stop it again when we're done.

That's really not a good idea, the API takes 2-4 minutes to launch. Just abort if it doesn't run, that'll be much more convenient.

penn@leela:~/libwf-java$ killall python*
python*: no process found
penn@leela:~/libwf-java$ killall python3.6
python3.6: no process found
penn@leela:~/libwf-java$ killall python3
python3: no process found
penn@leela:~/libwf-java$ killall python
python: no process found
penn@leela:~/libwf-java$ # No API Running
penn@leela:~/libwf-java$ arc unit
int(3)
int(3)
   PASS   1.6s   Unit Tests
   PASS   1.8s   SpotBugs
penn@leela:~/libwf-java$ # Now I'll start an API in the background
penn@leela:~/libwf-java$ killall python*
python*: no process found
penn@leela:~/libwf-java$ killall python3.6
python3.6: no process found
penn@leela:~/libwf-java$ killall python3
python3: no process found
penn@leela:~/libwf-java$ killall python
python: no process found
penn@leela:~/libwf-java$ # No API Running
penn@leela:~/libwf-java$ time arc unit
int(3)
int(3)
   PASS   1.8s   Unit Tests
   PASS   1.8s   SpotBugs

real	0m13.361s
user	0m12.339s
sys	0m6.544s
penn@leela:~/libwf-java$ # Now I'll start an API in the background
penn@leela:~/libwf-java$ pidof python3.6
32674 337
penn@leela:~/libwf-java$ # API is running
penn@leela:~/libwf-java$ time arc unit
int(3)
int(3)
   PASS   1.6s   Unit Tests
   PASS   1.7s   SpotBugs

real	0m3.595s
user	0m4.806s
sys	0m0.540s
penn@leela:~/libwf-java$

It takes exactly 10 seconds to start the API, it seems. I think this is negligible. (btw caches are dirty for both times, so no compilation was needed, only the actual testing)

arcgradle/__phutil_library_map__.php
7

It's generated as written in line 4, arcanist did it itself.

They sounds odd to me. I'll need to benchmark this on my machine to see exactly. You didn't forget to do the migrations, download the dependencies, etc?

In D218#5267, @CLOVIS wrote:

They sounds odd to me. I'll need to benchmark this on my machine to see exactly. You didn't forget to do the migrations, download the dependencies, etc?

Migrations are done automatically by api.sh
Downloading dependencies did add some overhead, api.sh takes 33 seconds to load according to time ./api.sh and control-C when it is started. However, if you're running the unit tests and they fail for no apparent reason, all it means is you have to start over, wait 33 seconds for the API, and rerun the unit tests. It is much faster to just start the API if its missing, especially as this avoids the overhead of starting gradle twice. And if you expect to be running the unit tests a lot, you can start the API in another window, removing the overhead.
The main reason for doing this is to prevent unexpected failures.

  • Don't kill the API if we didn't start it
  • Remove debug code and fix API running detection
This revision is now accepted and ready to land.Feb 9 2019, 9:39 AM

Is this ready for landing? Or do you plan to change other things

In D218#5293, @CLOVIS wrote:

Is this ready for landing? Or do you plan to change other things

All fine unless there's something you want changed.

This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.