Skip to content

Refactor descriptions and test for big numbers #227

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

Closed
wants to merge 1 commit into from

Conversation

NickNeck
Copy link

@NickNeck NickNeck commented Jun 1, 2018

That is just a little refactoring of the tests for big numbers. For my test-runner, I need unique names for descriptions. I think it is also a little bit more readable in this form.

@Julian
Copy link
Member

Julian commented Jun 2, 2018

Hey!

Thanks (again).

I'm not so comfortable with this to be honest :/

I'd love to resolve #223 first before merging these kinds of changes, for two reasons:

  • I don't want to cater to properties that we do not currently guarantee (like "all descriptions are unique") -- either we should make that so and add a (suite) test guaranteeing it is the case, or, I suspect more likely, you should change your suite loader :) -- the reasoning there is simple, either it's a guarantee going forward, or someone will introduce some later test that will again not have this property, and again we'll churn on it

  • At the minute it's a gray area on what's safe to change for backwards compatibility (The suite should explicitly document its backwards compatibility policy #223) -- all there is is the description essentially at the minute (which you can use as a fully-qualified-path to the test), so when you change that, anyone who might have pinned away from that test might now be broken (or well, might need to update their suite loader)

Hope that makes sense, but if not, lemme know, and definitely definitely would appreciate help on #223.

@NickNeck
Copy link
Author

NickNeck commented Jun 2, 2018

Hello,

that makes sense. I will tell my suite loader to be a little bit more relaxed.

Maybe I can write my opinions to #223 if I find some time.

Thanks, have a nice day.

@NickNeck NickNeck closed this Jun 2, 2018
@Julian
Copy link
Member

Julian commented Jun 2, 2018

That would be great :)

Thanks!!

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