The lead dev left the company two weeks ago. His last hurrah involved committing a bunch of complicated code into our API. The code he writes is generally overly coupled but this particular code is INSANE.
It is so coupled that the tests for it almost mock the entire application end to end. I only found this heap of garbage when the deployment tests were hung after I made a simple change in a totally unrelated file. THEY DIDNT FAIL. JEST GOT INTO A STATE WHERE IT CANT CONCLUDE AND HAS NO ERROR MESSAGES. We are taking about entirely different parts of the code. As far apart in the code as it can get. It took six hours of playing Sherlock Holmes to figure out what was breaking.

He got the most junior developer to approve the garbage PR as well.

I bet the junior is better than him....I bet MOST juniors are better than him. Now he can move on to another place where they’ll miraculously keep him around with a big salary, not knowing that he sucks and is shit for brains until he destroys their system like he destroyed yours with his shit code

Inform management, roll back his last commits, implement required functionality again, cleanly.

If management is against this idea, explain how everyone will otherwise spend X amount of time per week from now on fixing issues resulting from this poor architecture.

Oh another lead/senior developer that doesn’t know how to test and couldn’t care less about quality … Peter Law never fails.

@Maer I would roll it back but I have no idea why it works enough to reproduce the functionality. Also I don’t have time allocated to do that right now.

I also do know that it previously worked against an API for an external system that is now gone. 🙄

@irene I suppose the important part is to make management aware of the risks involved when using this code and build more code on top.

This obviously translates into cost down the line, something management should be able to understand.

@Maer That has been my overall impression of this product. The two lead devs I have worked with had a “push the chariot” mentality. That turns relatively simple functionality into very ugly code. I have communicated that to management before. I got “Not all tech debt is bad. Sometimes debt is how we pay for something that we can’t afford immediately.”

I agree with the sentiment but there have been a lot of of hot wet shit piles added to the code when we could have taken a few hours longer and engineered it properly. Until recently I was the only one on the team that uses an engineering approach.

Amusingly good unit test tend to test everything in the order of dependency so yeah they can get extensive

Sorry, I was in a rush.

@killames It tests an express router. There are five routes that point to four different controllers. Each route has auth middleware. The controller responses don’t matter in the test because that should be tested in the controller tests. The middleware rejections are important to test because they check the permissions on the routes. The liveliness of the routes and their params is also important.

What we ended up with is a router test that tests both the routes and controller responses. The controller responses depend on responses from the service layer. So the tests mock the service layer for the controllers that are in the test. They also mock a bunch of things in the controller. I made some changes in the service layer to add redis caching and the tests in the router choked because we aren’t mocking the redis connection so it attempted an actual connection to redis.

There is no defence for bullshit wide net coupled tests. Test the code you wrote.

@irene but arguably there is because it tests that something hasn’t been accidentally altered and can aid in changes you’re making or test code you’re busy writing

Can also tell you later in a sprawling million line app if everything is working to help you figure out what’s not

@killames Imagine if you were in school and writing a test for chemistry. The test is twice as long and the second half is about philosophy.

Angrily you go to the teacher and say, “This seems unfair that you would be testing philosophy on a chemistry test. They have nothing to do with each other.”

The teacher responds, “We heard that you were in philosophy class. That is also an important subject to know. So we are testing that on your chemistry test as well.”

You are arguing that the teacher has a valid point.

@killames Also I’ll point out what I wrote in the post. No tests actually failed in this situation. So how can this help find broken code?

@irene it fails once logic errors have been ruled out lol that or the test wasn’t written right or failed to pick up. A scenario

@irene that scenario doesn’t compute
Look in a perfect world a programmer takes an extra couple minutes to write something that tests his smaller code components like methods and classes

@killames It gets into a state where the test scripts simply don’t conclude. All tests pass. The test script never completes and there are no errors. That is why this is a rant. Scoping tests is important. Don’t couple the shit out of the system.

Basic engineering is basic.

@irene ohhh you mean he didn’t account for them in a time estimate ? Not my definition of coupling which usually means pair lol and I really don’t see how though. If the condition is correct only the uses of the items tested should be failing but there should be some small test for that too perhaps ?

Like let me put it this way
Let’s take the case of autogenned code which you autogen the unit tests for for say a dal layer
If any of the tests fail and manual inspection of the unit test code says it should work you know a table or store prof is missing

Image reading
You identify sections in an image comp v with a created image
Image pixels don’t check out you have a problem

Service testing
You start only the services in a pipeline pointing at a test environment you place a file or a record in a database somewhere you look at the way it’s changed

The last is where there could be some logic errors because maybe it creates. A record it shouldn’t

I’ve heard of unit tests that click on web interfaces

And always remember if time is built in its job security lol more time
To do everything

@irene I understand however I only ever got to write unit tests at one job and the problem is there’s were somehow running for several hours so they just commented them all out and never ran them prior to deploy

We have unit testing in our pipeline that happens when code gets promoted. We have functional tests in the repo that run with a PR or merge.

This is a functional test. Like seriously stop being such a testing fanboy and read the context here.

A "unit test" is intended to test a "unit". It is not intended to test dependency chains. "I need to test the app logic whole" is a non-argument, this is not what these tests are for.

I understand the intent to make sure the whole application works, and thus test whole chains of functionality, but even theory aside, I have seen such approaches in action and they blow up the larger the project gets. Unmaintainable, fragile and when it breaks you begin searching, because your test broke *somewhere* in a dependency chain.

This is also why unit tests tend to work with mock classes and frameworks such as Mockito. Your tests needn't to test objects plus dependencies, but only minimal units of functionality. You need to be certain what goes in and comes out is exactly what you want.

Whole modules are tested via end-to-end testing, integration testing, functional testing and so on.

@irene push the chariot mentality. This made my day thank you

@gcavalcante8808 Did you think about Plato’s allegory of the chariot? Or the song Roll The Old Chariot Along?

Plato’s.

@gcavalcante8808 This is why I like developers.