Page MenuHomePhabricator

GUI : Add 'TEST' tab and unit test implement
ClosedPublic

Authored by jsuya on Oct 30 2018, 11:00 PM.

Details

Reviewers
JackDanielZ
Summary

The test is compare the original shot that was already created(in .exu)
to the shot created in the local environment.
(I made the 'TEST' tab using the unused 'EA' tab. bowonryu recommended)

The test order(exactness unit test)

  1. Make test list.

We add exu to the test list.
The list item contains exu info, current state info, and checkbox for run test.

  1. Run test

We select the exu to test and start the run test.
The run test tests the selected exu items in order.
exactness_play -s -t ./eagen_xxxx_xxxx.exu -o ./eagen_xxxx_xxxx.exu.compare.exu
(Make new shot in the local environment)
exactness_inspect --compare ./eagen_xxxx_xxxx.exu ./eagen_xxxx_xxxx.exu.compare.exu
(Compare orignal shot to local shot)

Each step is displayed in item and exu compare result is displayed in item's icon.

Test Plan
  1. generate app
  2. generate scenario and validate
  3. open 'TEST' tab
  4. select exu and 'Add to unit test'
  5. select checkbox
  6. select 'Run test'

Diff Detail

Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8173
Build 7489: arc lint + arc unit
jsuya requested review of this revision.Oct 30 2018, 11:00 PM
jsuya created this revision.
jsuya edited the summary of this revision. (Show Details)Oct 30 2018, 11:01 PM
jsuya edited the summary of this revision. (Show Details)Oct 30 2018, 11:06 PM

Hi Junsu,

I didn't review all the code because I don't understand why you need to pack the binary inside. When the scenario is generated, the ML and the C code are stored inside the unit. The Exactness player knows how to deal with the C code (ML is just for ea_gen for display). Isn't it sufficient? Is this change motivated by the absence of a compiler in the environment?

Daniel

jsuya added a comment.Oct 31 2018, 1:08 AM

Hi @JackDanielZ
You are right.
The exactness_player don't need to binary.
I didn't know that
So I will remove the code related to binary packaging from this patch. thank you :)

jsuya updated this revision to Diff 17209.Oct 31 2018, 1:12 AM
jsuya edited the summary of this revision. (Show Details)

update code(remove the code related to binary packaging) and comments

jsuya updated this revision to Diff 17212.Oct 31 2018, 3:44 AM

update code

I know that my question can seem dumb but... why do you need threads? There is no calculation at all there, only pass to the main loop, meaning the threads mechanism won't block anything.

src/bin/gui.c
1992

What is -s for you? Doesn't it fail on your side?

You may need --show-only-diffs to display on the differences

I wanted compare to run in the background.
But there was no such option and exactness_inspect was not created that way.
-s option is wrong. I will fix it.

I think a thread will be needed when testing a large number of exu. (In fact, I followed the structure of app_gen.)
And now we simply call exactness_inspect. But in the future, the process of receiving the results and summarizing the results should be developed.

When we start the test, it is okay to run all the selected exu at once.
However, I think it's a good idea to run it appropriately according to the number of threads.

jsuya updated this revision to Diff 17214.Oct 31 2018, 11:05 PM

update code

+)
Currently there are no jobs that need to be blocked.
It seems that the part that did binary packing of the patch was that jobs.

There are currently no special ideas.
I have no idea how to display the results. But if we think about how to organize and display the results, we might need a thread.

I needed the threads to generate the ML files in parallel. I didn't want to do it externally because building the DB takes relatively a lot of time.

In your case, I don't see why you would need threads. The main loop already gives async, which is great imo with what you want to do.

Concerning the jobs stuff, as you may already know, there is a tests suite in Exactness. You provide a text file that contains the test name and the command to execute (this one is optional as the code may be in the exu itself). It already handles the jobs parallelism. The only point we would have to solve is to give results in live (Waiting, Run test, Compare, OK/FAIL with #errors). This could be solved by modifying exactness.c to print status on the screen (with some option to implement). Then ea_gen would just have to analyze the output of the exe (_exe_output_cb and _exe_end_cb), find the status messages ("STATUS: test_name -> TEST or COMPARE or OK or FAIL(3/7)), parse them and update the tests status in the GUI test panel. In this way, you delegate the work to Exactness itself. What do you think?

Concerning the results to display, I think giving the status as you do now is great. But when it is finished, show some OK/FAIL. When it is ok, you don't need to show the comparison as it is obviously the same. When it fails, add a button to show the comparison. I don't think showing it automatically is user friendly.

jsuya added a comment.Nov 2 2018, 12:46 AM

Hi

You are right. There is no reason to use threads. I will update it to not use threads

and
We need to modify the exactness_inspect to work without gui.
I would modify that after deciding how to make the comparison result.

And to talk about the results of the comparison
There are some ways with your advice.

  1. We use _exe_output_cb to get the result as if we got a log.

and parse them and update the tests status in the GUI test panel.
1-1. If item is selected, ea_gen is display in the preview tab as in _gui_unit_display function in inspect.c.
1-2. If item is selected, it is output to preview with winsocket and plug. (Is this possible?)

2.At the end of exactness_inspect --compare, make the .exuc file output.
This .exuc contains the results of the comparison of two exu and two exu.
At the end of the test, each item loads exuc and updates its status.
Select item to display the exec in the preview tab.

There is no problem in any way.
The first method will eventually run the _are_images_different twice in gui_unit_display (testing, preview)
However, the second method will be created new type file(exuc).

What do you think?

jsuya updated this revision to Diff 17222.Nov 2 2018, 12:47 AM

code update(remove thread part)

jsuya updated this revision to Diff 17223.Nov 2 2018, 1:06 AM

update code

Hi Junsu,

The inspector already supports non-GUI mode for comparison. If you add an output (-o), it will not launch the GUI. I just need to support the exu diff.

  1. Concerning the comparison display, it depends on what you want to display. If you want to show all the differences, it may be better to plug the inspector (with option --show-only-diffs). If you want to display only the different images (without the two compared units), it seems easier to read directly the output unit and to display the images.
  1. No exuc. I even don't think it is needed.
jsuya updated this revision to Diff 17270.Nov 5 2018, 10:31 PM

add compare view

jsuya updated this revision to Diff 17271.Nov 5 2018, 10:49 PM

support double click preview of exu compare result.

jsuya updated this revision to Diff 17272.Nov 6 2018, 12:05 AM

add hash data free and goto shot function.

jsuya added a comment.Nov 6 2018, 12:25 AM

@JackDanielZ

I know that using the -o option will create exu with image diff information. (Inspector: implement command line exu comparison)
but i do not use the output file. Of course I tried to use the output file. But I thought it would be better to recycle the existing code.
I applied the view of exactness_inspect to EXU_COMPARE preview.
and i will use that log information if the D7239 patch of @YOhoho is applied.

Maybe, I did not understand what you wanted.
Do you want diff preview to use the output file? But this is the same as the second I suggested.

If you have what you want, please tell me in detail.
If you do not mind, I want this patch to be merged soon.

What I meant:
For example, you need to run 100 tests together. You create some txt file with the tests name per line. You execute exactness with this file, -b the path to the exu files and other params that I don't remember. You attach to the Ecore_Exe some type "exactness_suite" or whatever you want. When you receive output in _output_exe_cb/_end_exe_cb, you parse to find some specific lines e.g STATUS test_20181107_1443_01 START... and other events such as COMPARE, OK, FAIL(3/7)... For each such event lines, you update your list item with the right status/color... That's why I don't understand why you need yohoho patch. His patch is usable only when the test is finished, but cannot help you to give live update. I just pushed the patch of the STATUS.
If the test failed, double-click can display the different images. To do this, you just need to open the exu output and show it.

jsuya updated this revision to Diff 17542.Nov 22 2018, 6:16 PM

I removed the code related to result preview. This is going to be a new ticket.
And I updated the code to fit the new layout.

jsuya updated this revision to Diff 17543.Nov 22 2018, 6:18 PM

remove unnecessary function

jsuya edited the summary of this revision. (Show Details)Nov 22 2018, 10:04 PM
jsuya updated this revision to Diff 17549.Nov 22 2018, 10:27 PM

update code

JackDanielZ accepted this revision.Nov 24 2018, 10:48 AM

I modified the code because it had issues on my side

This revision is now accepted and ready to land.Nov 24 2018, 10:48 AM
JackDanielZ closed this revision.Nov 24 2018, 10:48 AM