EPIJudge icon indicating copy to clipboard operation
EPIJudge copied to clipboard

Every time I push to my forked repo, an email with Test Fail is sent to me even when the tests passed

Open nonopolarity opened this issue 4 years ago • 3 comments

@metopa

I did the parity problem:

EPIJudge/epi_judge_python/parity.py:

def parity(x: int) -> int:
    p = 0
    while x:
        p = ~p
        x = x & (x-1)
    return p & 1 

and run it:

python3 parity.py 

all tests passed, and two files are changed. When I push:

https://github.com/jianlin/EPIJudge/commit/c94c77420f41a9c5e53a5b1fdbc50002de37922d#annotation_131280220131280220

Then an email with Error of Test Failed is sent to me:

Run failed for master (c94c774) Repository: jianlin/EPIJudge Workflow: Run Python programs Duration: 44.0 seconds Finished: 2020-03-04 23:36:28 UTC

View results

Jobs: 77513480-8bde-53f1-0bca-314e6f293bb3 failed (1 annotation)

The link to the Test Failed page:

https://github.com/jianlin/EPIJudge/runs/486327741

or

https://github.com/jianlin/EPIJudge/commit/c94c77420f41a9c5e53a5b1fdbc50002de37922d/checks?check_suite_id=499793460

nonopolarity avatar Mar 04 '20 23:03 nonopolarity

Hi @jianlin, thank you for reporting this.

It is a kinda funny and silly mistake on our side - our CI pipeline checks that all stub programs fail (to check that we don't have a working solution in stub programs). But this is not correct when you start adding solutions.

I'll fix this soon, for now just ignore these errors or disable "Test stubs" CI stage (remove lines 40-42 from .github/workflows/test_python.yml)

metopa avatar Mar 05 '20 10:03 metopa

Hi @jianlin, thank you for reporting this.

It is a kinda funny and silly mistake on our side - our CI pipeline checks that all stub programs fail (to check that we don't have a working solution in stub programs). But this is not correct when you start adding solutions.

I'll fix this soon, for now just ignore these errors or disable "Test stubs" CI stage (remove lines 40-42 from .github/workflows/test_python.yml)

@metopa thanks for checking this.

(oh you actually mean all files in epi_judge_python/ should fail the tests, but files in epi_judge_python_solutions/ should succeed? Then that means we could edit the stub files but save it to the solutions folder as parity-my-solution.py? ... except it won't update problem_mapping.js when we run the individual test)

I noticed that it seems when the Python program just return 0, then all tests will pass and the CI test will show an "ok". It is a bit strange that... is this the mechanism to consider the program "not written yet"? That's because, what if the solution attempted actually is returning 0 all the time (or the solution has a bug that always return 0)... then it'd be considered to be passing the tests too?

(so later on I found that the file there is by expectation to fail the test)

For example, if I intentionally add a "bug" to parity.py:

def parity(x: int) -> int:
    p = 0
    while x:
        p = ~p
        x = x & (x-1)
    p = 0
    return p & 1

this program is returning 0 all the time and the CI will actually consider the whole CI test passing.

I don't know of a better solution... maybe returning a string "Not Yet Implemented" can work but it may be an issue for C++ or Java (or Python) when the return type is specified to be not a string type. How about a comment that is

// IF THIS TOKEN:      NO_CI_TEST_WILL_RUN_FOR_THIS_FILE
// is present, then any CI test will not run this file (locally or on on GitHub)
// REMOVE THIS LINE AND THE ABOVE TWO LINES SO THAT THE CI TEST WILL RUN

Then other users can look at another user's repo, and grep for any file that doesn't have NO_CI_TEST_WILL_RUN_FOR_THIS_FILE and know the repo has an attempt to solve this problem.

Or one other solution... files in epi_judge_python/ will not run for the CI... but files in epi_judge_python_attempts/ will. And for the file to get recorded in problem_mapping.js, it has to be in epi_judge_python_attempts... that way, our solutions are all in epi_judge_python_attempts and tested in CI, and other users can look at what's in that folder to see what other people's solutions are. (we also can look at problem_mapping.js to see which problems the user did).

My suggestion:

And then... after trying more problems, I think the following is a simple way: files in epi_judge_python/ will not be in the CI test, UNLESS if the file is

sorted_array_remove_dups.answer.py

(having the answer.py ending)... So we can try and try our program, and when everything works, we can rename the file to having the answer.py ending... and then everything works... the change to the system is minimal...

This way, we can even have multiple solutions, like

sorted_array_remove_dups.first.answer.py sorted_array_remove_dups.second.answer.py

and they would got into the CI test.

Maybe you want sorted_array_remove_dups.py to be in CI, just to check that there is no syntax error, but just that the "return value" is not good. In that case, how about, the CI test will assert sorted_array_remove_dups.py runs but have bad results, but for sorted_array_remove_dups.answer.py it runs and have correct results.

nonopolarity avatar Mar 05 '20 10:03 nonopolarity

Right now this work flow seems to work fine:

edit epi_judge_python/sorted_array_remove_dups.py and run python3 <the-file.py> to test it.

When done,

mv epi_judge_python/sorted_array_remove_dups.py epi_judge_python_solutions/sorted_array_remove_dups.mine.py

it will get into CI and then, the stub is removed too so there is no problem.

nonopolarity avatar Mar 06 '20 06:03 nonopolarity