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

Fix inconsistent 'factor of x' description in raindrops #2330

Closed
wants to merge 1 commit into from

Conversation

BNAndras
Copy link
Member

All other descriptions referencing factor say 'factor x', but this test's description says 'factor of x' .

Copy link
Member

@IsaacG IsaacG left a comment

Choose a reason for hiding this comment

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

This should mostly not be visible to the student and changing this can cause churn in downstream track test files. Is there value in changing this? Does that value outweigh the consequences (making generated tests go out of sync)?

@BNAndras
Copy link
Member Author

I'm fine leaving it alone then.

@BNAndras BNAndras closed this Sep 24, 2023
@ErikSchierboom
Copy link
Member

This should mostly not be visible to the student and changing this can cause churn in downstream track test files. Is there value in changing this? Does that value outweigh the consequences (making generated tests go out of sync)?

I think the value is just consistency, and I'm totally fine with that. The amount of churn this generates is not much and we also don't requeue solutions when they are upgraded but only the tests file has changed, so there is little downside I feel.

@BethanyG
Copy link
Member

I think the value is just consistency, and I'm totally fine with that. The amount of churn this generates is not much and we also don't requeue solutions when they are upgraded but only the tests file has changed, so there is little downside I feel.

I am a bit confused here, because when a test file/test case changes, how do we know that the solution still passes? I would assume that that is the exact scenario when re-queueing would happen. Is that because the GUID stays the same, so the file is not technically "different" enough? Or is it a change that will need to be flagged at the track level to prevent re-queuing?

For the Python test generator, a change in description means a change to the name of the test in the test file, and I would assume a re-queue unless flagged. For this exercise, that's 17, 133 solutions. Fine to do that -- but it does seem a bit much considering the change is the removal of the word of.

@ErikSchierboom
Copy link
Member

Doh, I somehow misread 🤦 It does mean requeuing when the tests file change, unless the magic marker is present. Let's close this after all. My bad all!

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.

4 participants