-
Notifications
You must be signed in to change notification settings - Fork 266
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
BLADEBURNER: Test cover action completion #1695
BLADEBURNER: Test cover action completion #1695
Conversation
* | ||
* Example: 1e14 strength exp gives ~~831 strength skill without augs/modifiers | ||
*/ | ||
const HIGH_STAT_EXP = 1e14; |
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.
Where necessary to guarantee 100% success rate giving the player high combat stats through EXP.
const CITIES = <CityName[]>Object.keys(instanceUsedForTestGeneration.cities); | ||
|
||
/** All the tests depend on this assumption */ | ||
it("always succeeds with optimal stats, rank, stamina and city chaos levels", () => { |
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.
This is not a test directly, but rather a shared assumption that breaks all reliability should this one not pass.
Expect this to change only if we significantly alter balance in favor of making BB harder on combat stats, which I deemed unlikely.
}); | ||
}); | ||
|
||
describe("Upon successful completion", () => { |
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.
There's plenty of orthogonal concepts:
- the type of action: general, op, black op, contract
- failure vs success
- custom behavior
- black ops need setup
- contracts award money
To make test organization easier, I am using the Operation name where applicable to keep the tests in source code separate, but in the test runner they will merge according to the common describe block headers.
Ie. 2 separate tests for
- describe Recruitment behavior
- describe Recruitment stat gain
|
||
/** Stat EXP check for all actions */ | ||
/** Checking all of them to avoid regressions */ | ||
describe.each([...actionIdWithIndividualStat(nonGeneralActions())])("$id.name", ({ id, stat }) => { |
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.
This is the most complex bit. It generates pairs of Action + Stat depending on which stat weights are non-zero for each given action. This allows for a quick rundown of all possible combos to make sure we're not missing awards for things like hacking exp and int exp.
These are not hardcoded to the Actions per se, but using the data files through the static Bladeburner instance. It is somewhat coupled to implementation detail but I figured it was the lesser of two evils rather than writing a test suite of all the combinations manually.
function basicStats() { | ||
inst.rank = 1; | ||
inst.changeRank(Player, 10); | ||
Player.gainStrengthExp(LOW_STAT_EXP); | ||
Player.gainDefenseExp(LOW_STAT_EXP); | ||
Player.gainAgilityExp(LOW_STAT_EXP); | ||
Player.gainDexterityExp(LOW_STAT_EXP); | ||
|
||
inst.calculateMaxStamina(); | ||
inst.stamina = inst.maxStamina; | ||
|
||
resetCity(); | ||
} |
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.
Player setup where we want to keep numbers small in order to more precisely measure floating point operations (usually exp rewards).
function guaranteeSuccess() { | ||
inst.rank = 1; | ||
inst.changeRank(Player, 1e10); | ||
|
||
Player.gainStrengthExp(HIGH_STAT_EXP); | ||
Player.gainDefenseExp(HIGH_STAT_EXP); | ||
Player.gainAgilityExp(HIGH_STAT_EXP); | ||
Player.gainDexterityExp(HIGH_STAT_EXP); | ||
inst.setSkillLevel(BladeburnerSkillName.BladesIntuition, 1e12); | ||
inst.setSkillLevel(BladeburnerSkillName.Cloak, 1e12); | ||
inst.setSkillLevel(BladeburnerSkillName.DigitalObserver, 1e12); | ||
inst.setSkillLevel(BladeburnerSkillName.Reaper, 1e12); | ||
|
||
inst.calculateMaxStamina(); | ||
inst.stamina = inst.maxStamina; | ||
|
||
resetCity(); | ||
} |
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.
Player setup where we want actions to run as fast as possible (in simulated time) with 100% success chance
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.
Most of these PR comments should probably be code comments, if they're worth noting.
function guaranteeFailure() { | ||
inst.rank = 1; | ||
inst.changeRank(Player, 400e3); // Minimum to attempt to fail hardest op | ||
Player.gainAgilityExp(1e50); | ||
inst.calculateMaxStamina(); | ||
inst.stamina = 0; | ||
|
||
resetCity(); | ||
} |
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.
Guarantee failure on non-general actions by dropping stamina to zero.
inst.cities[inst.city].pop = 1e9; | ||
|
||
/** Disable random event */ | ||
inst.randomEventCounter = Infinity; |
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.
Random events trigger when the counter is <= zero
Don't want random events to interfere with checks for chaos, communities, pops or estimation figures
New testing PR up @d0sboots. Did a quick self-review. No code changes or early refactor this time. This relates to a few outstanding BB PRs where changes where deemed risky or need further testing. Hoping to get this merged sooner rather than later. To my knowledge, none of the tests are flaky. |
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 tests. Some coding/style stuff, but largely LGTM.
e8afd19 — covers all review feedback I believe. |
Cheers for going over it so thoroughly! |
}); | ||
}); | ||
}); | ||
|
||
it("have a minimum duration of 1 second", () => { | ||
complete(SampleContract); | ||
expect(inst.actionTimeToComplete).toBeGreaterThanOrEqual(1); | ||
expect(bb.actionTimeToComplete).toBeGreaterThanOrEqual(1); | ||
}); | ||
|
||
beforeAll(() => { |
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.
Optional: IMO it flows better if your setup code is moved to before the test code, but up to you.
BLADEBURNER: Test cover action completion
Things covered
Not covered in this PR
Outcomes
Bladeburner line coverage is now up to 60%. Branch cover is slightly lower than that due to the branches produced by overlaps with data and react components.
This PR makes it easier to perform changes in the following areas:
Linked issues
Documentation
No NS api changes or source changes in any production-facing code.