Page MenuHomePhabricator

Add arc unit engine for gradle
ClosedPublic

Authored by Hackintosh5 on Mon, Feb 4, 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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Hackintosh5 created this revision.Mon, Feb 4, 8:12 PM
Info-Screen requested changes to this revision.Mon, Feb 4, 8:17 PM
Info-Screen added inline comments.
.arclint
29

This linter should be the only one removed

This revision now requires changes to proceed.Mon, Feb 4, 8:17 PM
Hackintosh5 updated this revision to Diff 765.Mon, Feb 4, 8:21 PM
  • Remove debug code
Harbormaster completed remote builds in B567: Diff 765.
Hackintosh5 updated this revision to Diff 767.Mon, Feb 4, 8:36 PM
  • Bring back the linters
Hackintosh5 marked an inline comment as done.Mon, Feb 4, 8:38 PM
Info-Screen accepted this revision.Mon, Feb 4, 8:39 PM
This revision is now accepted and ready to land.Mon, Feb 4, 8:39 PM
CLOVIS added inline comments.Tue, Feb 5, 6:35 AM
arcgradle/unit/GradleUnitTestEngine.php
44

You have SpotBugs twice, is that intentional?

CLOVIS added 1 blocking reviewer(s): CLOVIS.Tue, Feb 5, 6:36 AM
This revision now requires review to proceed.Tue, Feb 5, 6:36 AM
CLOVIS added a comment.Tue, Feb 5, 6:40 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.

Hackintosh5 added inline comments.Tue, Feb 5, 9:50 AM
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

Hackintosh5 updated this revision to Diff 769.EditedTue, Feb 5, 12:58 PM

Push accidental code, now removed

Hackintosh5 updated this revision to Diff 770.Tue, Feb 5, 1:06 PM
  • Autostart the development API if it's not running already, and stop it again when we're done.
Harbormaster completed remote builds in B572: Diff 770.
Hackintosh5 updated this revision to Diff 771.Tue, Feb 5, 1:40 PM
  • Remove debug and unused code
Harbormaster completed remote builds in B573: Diff 771.
CLOVIS added a comment.Tue, Feb 5, 2:41 PM
  • 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.

CLOVIS added inline comments.Tue, Feb 5, 5:52 PM
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 marked an inline comment as done.Tue, Feb 5, 8:08 PM
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?

CLOVIS added inline comments.Tue, Feb 5, 8:13 PM
arcgradle/__phutil_library_map__.php
7

How and when is it generated?

Hackintosh5 marked an inline comment as done.Tue, Feb 5, 8:16 PM
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)

Hackintosh5 updated this revision to Diff 773.Tue, Feb 5, 8:20 PM
  • Cleanup code
Hackintosh5 added inline comments.Tue, Feb 5, 8:26 PM
arcgradle/__phutil_library_map__.php
7

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

Hackintosh5 marked an inline comment as done.Tue, Feb 5, 8:27 PM
CLOVIS added a comment.Wed, Feb 6, 7:03 AM

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?

Hackintosh5 added a comment.EditedWed, Feb 6, 8:23 AM
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.

Hackintosh5 updated this revision to Diff 774.Wed, Feb 6, 11:01 AM
  • Don't kill the API if we didn't start it
Hackintosh5 updated this revision to Diff 775.Wed, Feb 6, 11:06 AM
  • Remove debug code and fix API running detection
CLOVIS accepted this revision.Sat, Feb 9, 9:39 AM
This revision is now accepted and ready to land.Sat, Feb 9, 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.