Skip to content

J-term 2018 and 2019#38

Open
Yook74 wants to merge 322 commits intogeisler:masterfrom
Yook74:master
Open

J-term 2018 and 2019#38
Yook74 wants to merge 322 commits intogeisler:masterfrom
Yook74:master

Conversation

@Yook74
Copy link
Contributor

@Yook74 Yook74 commented Jan 22, 2019

Features Implemented:

  • Cronscript Rewrite
    • The cronscript is now written in Python
    • During the rewrite we made several large changes to the structure and organization that should drastically improve maintainability.
    • We also know that the cronscript works flawlessly because of the tests.
  • Tests
    • A full suite of tests have been written.
    • To our knowledge, every somewhat working feature of the programming contest server has been tested.
  • PHP 7
    • The codebase has been upgraded from PHP 5 to PHP 7.

Issues resolved:

We also fixed quite a few bugs not mentioned in the issues

Yook74 added 30 commits November 1, 2018 11:15
The old script was unsuccessfully trying to copy all of usr/bin/x86_64... into the jail.
This new version only copies the standard c++ library from that directory
It is also heavily refactored to look more like the new java jail script.
At this point the contest is pretty much working.
Most of the basic features have been manually tested.
There is one known issue: The judge UID and GID are hardcoded in chroot_wrapper.
These values must be changed for every contest.
node_modules, vendor, and composer.* were in the repo, and Santa almost had to kill a kitten.
Lickily the crisis was averted and the binaries were removed and added to .gitignore.
I tested the install to make sure it still works and it does.
Instead of categorizing tests by the user, I want to categorize tests by feature and put user-specific commands in actor classes.
So I split up the existing AcceptanceTester code into three classes and did some refactoring.
More functionality definitely needs to be added but this is a start.
I took the existing tests, restructured them, and added new stubs.
I did a lot of moving around and refactoring.
Some deleting too.
I'm intending to do this to the admin tests as well but I don't have time at the moment.
I didn't want to call login() in every test so I decided to put it in the class constructor.
I moved more of the admin tests into the structure I'm using.
They aren't fitting super well though. I will probably change it up some in the near future
Geisler and I decided that all the setup for every test should exist in the actor classes.
I started moving all the setup code for the ClarificationCest to the Judge and Team Actors.
Calling your parent's constructor in your constructor is important and I forgot to do that earlier
Now the three actors have a parent class, AcceptanceTester.
AcceptanceTester automatically loads attributes from the various .ini files for each type of actor.
The .ini files contain things like the name of the contest and the various actor's usernames.
This includes adding a method and a few attributes to CreatorActor.
Untested
This was necessary because the tests can't connect to the database immediately because it doesn't exist until we create it in CreateCest
The thing I tried last didn't end up working.
The current solution is somewhat hackish but at least it's automatically invoked.
See Helper/Acceptance for more info
dthomas008 and others added 3 commits January 23, 2019 17:01
Copy link
Owner

@geisler geisler left a comment

Choose a reason for hiding this comment

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

  • Let's delete develop/old_chroot_wrapper.c, develop/start_contest.crontab, develop/stop_contest.crontab as part of this commit.
  • I like exec_and_check() in lib/create.inc
  • There are a lot of files with new permission of 755 that should have stayed 644 since they aren't directly executable. For example, public_html/admin/email_body.txt.
  • In update_database(), do the TRUNCATE table commands reset the autoincrementing IDs for the table? If not, I would like to find a way to ensure the cloned contest IDs start at 0.
  • Let's not keep line 196 of public_html/admin/setup_contest.php since it is just commented out debugging print out.
  • There appear to be some lines that are longer than 80 characters. That is a personal pet peeve of mine because I often log in to the system with a console of exactly 80 characters and the line wrap is visually annoying. An example would be line 117 of public_html/admin/setup_headers.php.
  • What does the tag garph 2019 in public_html/admin/setup_team_category.php line 70 mean?
  • public_html/judge/errorLog.txt should not be in the repo.
  • Line 31 of public_html/judge/py_lib/c_submission.py should probably have its regular expression changed to r'(#include\s+["<][^">]+[">])'
  • Similarly, the regular expression in public_html/judge/py_lib/java_submission.py should probably be changed to r'(import\s+[^;]+;)'.
  • The public_html/judge/py_lib/c_config.ini file should probably add -fno-asm to its compiler flags to match the CXX compiler flags.
  • I'm leery of having -o in the compiler flags for C/C++ because it needs to immediately proceed the executable name in the command line. This is not an optional flag that can be set or cleared by someone configuring the software and could carelessly be removed. I think this flag should be explicitly added to the compile command since it must always be there and always be in that exact location in the command.

@dthomas008
Copy link

On your fourth comment, here's documentation stating that it does reset the auto incrementing IDs: https://oracle-base.com/articles/mysql/mysql-how-truncate-table-affects-auto-increment

Yook74 and others added 4 commits January 24, 2019 08:33
Removed accidental joke comment
I removed -o from the config files and added it into the code so that it could not be accidentally removed or misplaced.
-fno-asm was also added to the c compiler flags
@Yook74
Copy link
Contributor Author

Yook74 commented Jan 24, 2019

On the line length issue: There are quite a few lines in the repo that exceed 80 characters. We were using a limit of 120 characters. Would you like us to go through the lines we committed and shorten them to 80 characters?

Yook74 and others added 6 commits January 24, 2019 09:36
calling the cronscript with the --rejudge flag will recalculate all the responses in the AUTO_RESPONSES and JUDGED_SUBMISSIONS table
The queue ID was being put in the AUTO_RESPONSES table instead of the judged id
This flag takes the human judge out of the loop and assigns final judgements to queued submissions
These added features are currently not exposed to the users, so they're not truly new features yet.
The important thing is the bugfixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants