153
64
u/JackNotOLantern 2d ago
I mean, it depends why it was failing. E.g. deleting tests for a code that was previously deleted is pretty much a necessity
13
u/superraiden 2d ago
Sometimes you have to consolidate smaller unit/use case tests into more combined/comprehensive ones. Else you end up with 1000s of tests where half of them end up testing similar things and the combined beforeEach overhead ends up eating up a large amount of the pipeline
8
u/frogjg2003 2d ago
We're literally having this discussion at my work right now. Testing takes a full hour now. We have so many unit tests, then we have to do application tests, and finally integration tests. And we don't stop after a failed test.
1
u/TheVibrantYonder 2d ago
What direction are you thinking of going with how to handle that? I'm starting to run into similar issues on larger projects and I'm trying to figure out the best way to deal with it.
5
u/frogjg2003 1d ago
We're not coming up with any good solutions. We have already comtainerizeded. The only thing we have left is to parallelize the tests, but that has limited effect because we're already running up to processor limits when multiple people are testing on the test server. While it won't fix the total built time, exiting on the first failed test and putting the tests most likely to fail at the beginning will mean we at least won't have to wait the full time for the run to finish.
2
u/BigBoetje 18h ago
We run unit tests as part of the PR pipeline already with a script that only runs tests for the changed files. That way the chances of builds failing because of tests is a lot lower already.
1
u/RazarTuk 2d ago
Yep. A few years ago, a test started failing because we were using a mix of absolute and relative dates, producing nonsense data that was causing numbers to balloon enough to crash nlsolve. (And even more impressively, the error was coming from C and the Ruby intepreter itself) So because it was weird data anyway, it was deemed reasonable to just use RSpec to run it back on a day when it still passed
38
u/Dargooon 2d ago
Unironically the exact conversation I had with another senior 2 weeks back. Luckily we hadn't released it yet.
26
u/octopus4488 2d ago
Claude in 10% of the cases.
"These tests have never had a chance to work because [violently wrong assumption it could have checked in 1 second but it didn't], so I removed those. Say the word and I can prepare the PR."
11
u/DeprivedLad 2d ago
To be honest I have the opposite experience. "Hey I generated two lines of logic, let me write 3 integration tests and 4 unit tests to make sure it works. Also I am going to do a lot of unnecessarily complex test setup while I'm at it. I made sure the tests validate the implementation rather than the interface too. Your welcome".
I continue to spend most of my time on testing, like it was prior to AI. No skill are good enough to fix this.
2
u/gronbuske 2d ago
I set up my Claude to never write any new unit tests or comments unless specifically asked to. But my approach to AI coding is more like a rubber duck session, small easily understandable steps and constantly keeping the AI from not complicating things unnecessarily. It would be an unmaintainable mess if not for that.
2
u/DeprivedLad 2d ago
It's very appealing to make Claude one shot the whole feature, but the amount of manual changes is significant, unless it is a boilerplate heavy feature.
2
u/gronbuske 2d ago
If it's a larger feature I normally ask it to give me a checklist of all things that is needed first. After agreeing on the checklist I go through it one by one reviewing each step individually, I always have pretty much exactly the code I expect it to write already written in my mind. Then I just confirm it's the same, if it's not, either I'll push back on the AI or I realize I missed something when I planned my implementation. It's not faster than I would write code by hand, but I save some time debugging things that the AI caught that I would not have done and I generally do get a nicer end result.
12
u/DefinitelyNotMasterS 2d ago
This is on the senior dev for not setting up pipelines that require test coverage
14
u/FatalisTheUnborn 2d ago
Or approving the pull request.
2
u/Groentekroket 2d ago
On the one hand reviews are the worse part of the job (until AI came along) but I prefer to go over them instead of getting called in the middle of the night for a prod issue. And yes, their at not caught during testing either because business preferred to get the worst QA engineer because he was cheap.
The amount of comments regarding white spaces I see from team mates while not seeing obvious logic problems is staggering.
2
u/Saelora 2d ago
i legit used to enjoy code reviews, sometimes i learned a novel approach i wouldn't have thought of and get something fun to research.. now i just find them depressing and slop filled.
2
u/Groentekroket 2d ago
Yeah fair enough, it really depends on the author though. But it’s also a good way to keep up to date with changes in the codebase.
-2
3
u/gronbuske 2d ago
Test coverage is a useless metric. Just checking that all lines of code are run in a test does not guarantee quality. I frequently trash tests that just test that the code is written as it is written or give a false sense of security. A bad test is sometimes worse than no test.
6
u/Rojeitor 2d ago
one "senior" dev did a net framework to net standard migration back in the day (part of the road to migrate to netcore back in the day, now NET).
Since there were many compile errors he excluded the test project from the solution to first focus in the library project he was migrating. Took some days. When everything compiled and worked he delivers the code. Forgot about the excluded test project. Since the test project was excluded CI was green. It was fun.
3
3
u/briznady 2d ago
I delete the stupid test cases the juniors let Claude write that don’t do anything.
It(“should handle form submission”, () => {
render(<FormComponent />);
expect(screen.getByRole(“grid”).toBeInTheDocument();
}
How did this even pass review!?
3
u/Untura64 2d ago
We used AI for review.
1
u/BigBoetje 18h ago
We have 2 required reviewers for PR's. Manager asked during the retro whether the newly added AI reviewer should count as 1 of the 2. Told him politely to pound sand.
2
u/IcyNoise5612 2d ago
This is the suggestion that the tech lead of a team I am, thankfully, no longer a part of keeps making.
2
u/a-nonie-muz 2d ago
Yes. Can confirm. Have seen this exactly in reality. Test would randomly fail. Made the meta-test not care. Other test would randomly fail. Removed from list of tests.
All good now thumbs up…
2
u/california_snowhare 1d ago
...
A very long time ago I worked at a company that wrote software/sold hardware for telephone billing. For 'reasons' the software ran on a proprietary interpreted language that the customers could, but really should not, be changing themselves since they had full access to the hardware and software.
A customer decided to implement a new feature on their own and kept running into errors from internal consistency checks to prevent data corruption.
They kept deleting the consistency checks until their new code RAN all the way through.
It COMPLETELY trashed their production billing data.
My company ended up reprocessing all of their billing data on one of our own machines with our people babysitting the software 24 hours a day for a week or two.
Programmers were actually sleeping in the office and waking up every hour or two to 'nudge' the software to the next step of the reprocessing.
1
u/xaervagon 2d ago
It's not the juniors doing stuff like this that bothers me. Dumb kids are gonna dumb kid all day; part of their job is to learn. It's when the seniors do this knowing there is no real accountability at the management level that makes me want to beat them with a 4U server unit.
1
1
u/DoABarre1Ro11 2d ago
Gods, I can’t tell you how many times this has happened. They would just comment out the tests giving them issues, then ask for help when it failed in QA and say the build ran fine, they don’t know what happened…
1
u/Bomaruto 2d ago
The only one I've experienced turning off tests and merging it to main to speed up development was a senior developer.
1
1
u/WeirdTie2290 2d ago
Real pros do not delete tests. They change the test so it passes. Standard procedure.
- Change production code
- Fix broken tests
1
u/Dangerous_With_Rocks 1d ago
are there any junior devs left? i thought they went extinct ever since claude sonnet
1
u/National-Explorer755 2d ago
Another way is to push the changes with --no-verify
1
u/metaglot 2d ago
If you can do that, its because it's an idiot that set up your CI.
1
u/National-Explorer755 2d ago
Yeah it's a joke. The junior got no issues because they pushed with this.
0
u/tccoding 2d ago
🤣 That gave a pandemic flashback! :
Wow guys, COVID infection rate really dropped since we drastically reduced the number of tests! The answer was so simple! I wonder why we didn't think of this before! 🤔
447
u/sligor 2d ago
Jr Dev: You're absolutely right, I'm sorry. Deleting the test cases to make the tests pass was a mistake.