Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test Cases Improvements #2298

Closed
wants to merge 0 commits into from
Closed

Test Cases Improvements #2298

wants to merge 0 commits into from

Conversation

FherStk
Copy link
Contributor

@FherStk FherStk commented Dec 22, 2023

Dear DMOJ team,

I developed the following (in my opinion) improvements:

Test cases (edit form):

  • When entering on 'edit test cases', if no data has been entered through the site but the test-cases have been created manually into the file system, the file-system data will be loaded automatically to maintain the website/file-system consistency. This loading process is performed by parsing the YML file, useful when the problems are created using a script (for example, when migrating from other platforms into DMOJ).
  • When selecting 'clean the zip file', all the test cases will be also removed to keep the consistency between the displayed data, the database and the YML file.
  • A new dropdown option called 'Overwrite test cases from zip file' has been added (disabled by default). If enabled, the test-cases will be automatically loaded and created using all the zip file entries (using default data, zero points also). Useful to quickly load all the test-cases from a zip:
    image

Problem (edit form and statement):

  • A new option called 'Include test cases' has been added to the 'edit problem' form (disabled by default, no pre-tests will be shown):
    image
    When enabled, the current test-cases will be displayed after the problem statement (the content of the in/out files within the zip file will be used). This data is being stored into the database, in the problem_data model:
    image
    Including the test-cases input and output manually in the problem statement is still possible, just non-enabling this new option, but this option is useful to avoid inconsistencies between the stored test files and the displayed data.
  • A new option has been included for attribution purposes, in order to give the authorship to external DMOJ users (like websites, books, etc):
    image
    This data will be displayed at the end of the problem statement, just before the license:
    image

General:

  • Added new django settings to disable social and comments sections (false by default):
    image

Social and comments enabled (by default):
image

Social and comments disabled:
image

@codecov-commenter
Copy link

codecov-commenter commented Dec 22, 2023

Codecov Report

Attention: 116 lines in your changes are missing coverage. Please review.

Comparison is base (2035e60) 46.75% compared to head (eb0a63e) 46.41%.

Files Patch % Lines
judge/models/problem_data.py 6.86% 95 Missing ⚠️
judge/views/problem_data.py 10.00% 9 Missing ⚠️
judge/views/problem.py 0.00% 8 Missing ⚠️
judge/views/blog.py 0.00% 2 Missing ⚠️
judge/views/contests.py 0.00% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2298      +/-   ##
==========================================
- Coverage   46.75%   46.41%   -0.34%     
==========================================
  Files         249      250       +1     
  Lines       13232    13365     +133     
==========================================
+ Hits         6186     6203      +17     
- Misses       7046     7162     +116     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Xyene
Copy link
Member

Xyene commented Dec 25, 2023

Thanks for the contribution! Could you please split up the "test case" / "problem" / "general" features in separate PRs, so that they may be reviewed and merged independently?

@FherStk
Copy link
Contributor Author

FherStk commented Dec 25, 2023

Thanks for the contribution! Could you please split up the "test case" / "problem" / "general" features in separate PRs, so that they may be reviewed and merged independently?

Sure! Which way would be the best for you: incremental PRs or independent PRs?

Thanks again and have a nice holidays 😊
Fher

@Xyene
Copy link
Member

Xyene commented Dec 25, 2023

Independent PRs would be best. Some of these changes I think we could ~straightforwardly merge:

For the others:

  • When entering on 'edit test cases', if no data has been entered through the site but the test-cases have been created manually into the file system, the file-system data will be loaded automatically to maintain the website/file-system consistency.

I think this shouldn't live in the site UI. The YAML format we use allows embedding Python code that is evaluated to construct YAML nodes (this is useful for implementing problems with random test case orders, for instance), and such problems inherently can't be represented in the site UI.

It may make sense as an import-simple-problem-type management command though?

  • When selecting 'clean the zip file', all the test cases will be also removed to keep the consistency between the displayed data, the database and the YML file.

  • A new dropdown option called 'Overwrite test cases from zip file' has been added (disabled by default). If enabled, the test-cases will be automatically loaded and created using all the zip file entries (using default data, zero points also). Useful to quickly load all the test-cases from a zip ...

I like this idea, but maybe we could make the UI a little simpler: have a single checkbox be "Infer test cases from zip", and if checked it blows away any existing test cases and repopulates them from the zip. Bonus points if it keeps any assigned point/pretest/etc. values from a preceding upload for cases named the same.

@FherStk
Copy link
Contributor Author

FherStk commented Dec 25, 2023

Hi,

Thanks again for your feedback, it's always a pleasure to collaborate with you.

About:

I think this shouldn't live in the site UI. The YAML format we use allows embedding Python code that is evaluated to construct YAML nodes (this is useful for implementing problems with random test case orders, for instance), and such problems inherently can't be represented in the site UI.

Maybe I can just display a warning (using the feedback field) with links as options, something like:
image

I'm not sure how to implement commands like 'import-simple-problem-type', but I can do it also and call it internally if you like from the warning options.

Thanks again,
Fher

@FherStk
Copy link
Contributor Author

FherStk commented Dec 27, 2023

Independent PRs would be best. Some of these changes I think we could ~straightforwardly merge:

For the others:

  • When entering on 'edit test cases', if no data has been entered through the site but the test-cases have been created manually into the file system, the file-system data will be loaded automatically to maintain the website/file-system consistency.

I think this shouldn't live in the site UI. The YAML format we use allows embedding Python code that is evaluated to construct YAML nodes (this is useful for implementing problems with random test case orders, for instance), and such problems inherently can't be represented in the site UI.

It may make sense as an import-simple-problem-type management command though?

  • When selecting 'clean the zip file', all the test cases will be also removed to keep the consistency between the displayed data, the database and the YML file.
  • A new dropdown option called 'Overwrite test cases from zip file' has been added (disabled by default). If enabled, the test-cases will be automatically loaded and created using all the zip file entries (using default data, zero points also). Useful to quickly load all the test-cases from a zip ...

I like this idea, but maybe we could make the UI a little simpler: have a single checkbox be "Infer test cases from zip", and if checked it blows away any existing test cases and repopulates them from the zip. Bonus points if it keeps any assigned point/pretest/etc. values from a preceding upload for cases named the same.

Hi there,

I completed the independent PRs:

I keep this PR closed, as it became outdated.
Please, do not hesitate contacting me if you need further information or additional changes, it's a pleasure to collaborate with this project :)

Kind regards,
Fher

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants