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

Store the results of running the question tests in the database #360

Closed
timhunt opened this issue Mar 19, 2018 · 8 comments
Closed

Store the results of running the question tests in the database #360

timhunt opened this issue Mar 19, 2018 · 8 comments

Comments

@timhunt
Copy link
Member

timhunt commented Mar 19, 2018

Even with the CAS cache, running question tests can be quite slow, and then the results are only shown on-screen, which is ephemeral and the data is not easy to use.

I propose to add a new database table which just permanently stores the result of each question test when it is run, and is also easier to query. I happen to need this for what I am currently doing, but it seems generally useful enough that I plan to submit a pull request. The proposed table structure is:

qtype_stack_qtest_results
    id     autoincrement
    questionid int \ Foreign key, references qtype_stack_qtests (questionid, testcase)
    testcase   int /
    seed       int
    restult    bool
    timerun    int (timestamp)

I will also add a timemodified column to qtype_stack_qtests, and ensure it is updated correctly.

Of course, once this is done, one could imagine all sorts of other clever features build on top of it (e.g. reporting, use the stored results rather than re-running, if the timestamps show that is valid, ...) However, I do not propose to do any of that now.

@timhunt
Copy link
Member Author

timhunt commented Mar 19, 2018

This is my proposed change. Comments? iss357...store-question-test-results

@timhunt
Copy link
Member Author

timhunt commented Mar 19, 2018

Note, this is not fully tested yet. Use with care!

@aharjula
Copy link
Member

Caching those results in the database for easy querying and correlation is a good idea and should have been done from the start so no opposition from here.

Should probably drop the contents of that table when upgrading STACK version, if I understood correctly you rerun tests only if the test changes and you probably want to run them also when STACK changes.

The ordering of the tests should not break this when we store the ordering like we do, or could there be a possibility of dropping a test from the list and the matching cache items shifting to the next test, does not seem like so by just looking at the code and I assume you have tested that already.

Could you at the same time add to the qtype_stack_options a caching field for static question validation as described in #339. Basically, no need to rerun the whole question validation if nothing can change and if we store the result in the database we can quickly list those questions that have validation issues. Essentially, would be a part of the same future reporting of question health and would similarly make it (and other things) faster.

@timhunt
Copy link
Member Author

timhunt commented Mar 19, 2018

No, I do not "rerun tests only if the test changes".

In this commit, there are no changes in how or when the tests are run, and I don't want feature-creep in this one change.

As I said, once this change is done, other things can be done on top of it. All the things you suggest are probably good ideas, but later.

@aharjula
Copy link
Member

Ok. So I assumed that that is the obvious use case for that "timerun" column. Does it have some other use for existing? In any case including that cache clearing now will avoid missing it later and forgetting that will cause a mess.

@timhunt
Copy link
Member Author

timhunt commented Mar 19, 2018

That is one of the obvious use-cases for that value. For now, it is mainly so that when querying the strored data, you can include condidtions like question.timemodied < textresult.timerun, etc. Anyway, the point was that it seemed like an importnat bit of metadata to save.

@timhunt
Copy link
Member Author

timhunt commented Mar 23, 2018

I have been using this for a few days, and I think it is working.

I'm seeing Chris on Sunday, I guess we might talk about merging this then.

@sangwinc
Copy link
Member

Thanks for this Tim,
A really valuable addition, as with the previous modifications to the bulk scripts.

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

No branches or pull requests

3 participants