-
Notifications
You must be signed in to change notification settings - Fork 316
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
Add mutation testing to CI #156
Conversation
3762ce6
to
d1e39a5
Compare
c51abb4
to
81dda36
Compare
b3b5be8
to
8605a05
Compare
1b1d3f9
to
ca2079b
Compare
so, with those changes we end up with 222 test suites executed (6.7s per test suite execution), better, but not really good, that still means a 99.9% confidence interval of 10 percentage points looks like we need to get down to around 1s per test suite execution to get workable confidence intervals |
b8d3d3f
to
967ed08
Compare
…cute at least once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only nitpicks. I did no spotted any obvious flaw in the changes.
systemd-run --user --scope -p MemoryMax=2G -p MemoryHigh=2G cosmic-ray --verbosity INFO exec cosmic-ray.toml session-vs-master.sqlite & | ||
cosmic_pid=$! | ||
for i in $(seq 1 600); do | ||
# wait for test execution at most 10 minutes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: Isn't it easier (if possible) to use timeout 10m?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea, but to implement it I would have to figure out error handling and so on... this is good enough (and probably easier to port to macos)
with self.assertRaises(ValueError) as e: | ||
SigningKey.from_secret_exponent(1234567890, curve=Ed25519) | ||
|
||
self.assertIn("don't support setting the secret", str(e.exception)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: exception message sounds a bit odd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's because this isn't the whole message, the whole message is "Edwards keys don't support setting the secret scalar (exponent) directly"
except ImportError: | ||
try: | ||
from gmpy import mpz | ||
except ImportError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it safe the second ImportError to be completely silent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why it wouldn't be? the point of this code is to define the mpz()
method, where it comes from is secondary
related to #131
0.14.1: With just 20 mutants executed in 25 minutes, it's not really useful (the confidence interval is way too large)
0.15-beta (8deb089): Single test suite execution takes about 9.97s on Travis, in 25 minutes we execute 107 tests, so better, but that still translates to a 99.9% confidence interval of 5.39% to 26.38% for survival rate. So we will need a faster test suite still.
0.16.0: No significant change, about 152 tests in 25 minutes,
todo:
@slow
decorator (pytest docs)@slow
decorator