Skip to content
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

The build has been broken since Jan 23rd 2013 #1754

Closed
evjan opened this issue Sep 15, 2013 · 23 comments
Closed

The build has been broken since Jan 23rd 2013 #1754

evjan opened this issue Sep 15, 2013 · 23 comments

Comments

@evjan
Copy link

evjan commented Sep 15, 2013

There hasn't been a green build in over 7 months. That's over 226 red ones.

I suggest the broken code is fixed or the build is removed from Travis and the README (since nobody seems to care about it anyway).

@jonathanong
Copy link
Member

Do you know what's wrong? This comment isn't really helpful because everyone can see that it is failling. We just haven't had the time to figure out what exactly is wrong.

@evjan
Copy link
Author

evjan commented Sep 16, 2013

I don't know what's wrong, I created this issue to highlight that it seems like a cultural problem that 226 builds in a row have been red and nobody seems to care.

The culture in this project seems to be "create new functionality instead of taking the time figuring out what is broken in our app". If so, I think the build (and perhaps even the tests) should be removed, because right now, if you'd want to start helping the project, you'd get stuck instantly since it's broken and you don't know if it's supposed to be that way or not.

@jonathanong
Copy link
Member

removing tests is a terrible idea, and the nice part about CI is that you can see exactly which tests are failing.

@evjan
Copy link
Author

evjan commented Sep 16, 2013

I'm sorry, I must be explaining myself poorly. I'll try to rephrase my thoughts.

If "the nice part about CI is that you can see exactly which tests are failing", then I assume you check after every build that the same tests are failing every time?

For me, the main benefit of CI is that you see an error quickly and then the team doesn't commit new stuff until that error has been removed. I pretty much agree with the practices and responsibilities as outlined here: http://www.thoughtworks.com/continuous-integration

If you like the status quo, then I won't argue with you. For me the problem was that I wanted to contribute to Express, but given that the code already is broken, it is very hard for me to tell if my changes broke something or not.

I could help fixing the broken build, but IMO the root problem is that nobody seems to care, so the build would probably become red again soon for some other reason. I wanted to create a discussion around that.

@jonathanong
Copy link
Member

I assume you check after every build that the same tests are failing every time?

yes. not particularly difficult to do.

nobody seems to care

false assumption. you care. are you saying you're nobody? the issue is that nobody has the time to fix it. if you have time and figure out what's wrong, please fix it. i already spent some time a while ago and couldn't figure out what is wrong. this is open source, blaming other people only makes yourself look bad.

@tj
Copy link
Member

tj commented Sep 16, 2013

it's just a bug in the tests, I should have fixed it a long time ago but yeah, such is life. Lesson learned: don't merge tests using mocks

@evjan
Copy link
Author

evjan commented Sep 16, 2013

I do care, yes. I wanted to highlight a cultural problem, not blame individuals. The former is a good thing, but trying to find scapegoats is not. I'm apologise if you felt like I was trying to blame people, I just wanted to lower the barrier of entry for new contributors (like myself).

That's also why I've now created a separate issue for the bug itself, this is more to discuss and learn from the past.

@rummik
Copy link

rummik commented Sep 16, 2013

I'd like to point out that I've been wary of updating because of the red indicator from Travis, and I'm guessing other people may be the same way

@tj
Copy link
Member

tj commented Sep 16, 2013

we have to find which test(s) are manipulating the prototypes, the tests reported are not even where the issue originates

@rummik
Copy link

rummik commented Sep 16, 2013

Probably some code in the commit where the build started failing? :)

@tj
Copy link
Member

tj commented Sep 17, 2013

I'd imagine :p thanks for the non-help

@rummik
Copy link

rummik commented Sep 17, 2013

I'm looking, but the only one I see failing is reported in #1758. It sounded like there were more tests failing?

@sorribas
Copy link
Contributor

Yeah, there is another one failing in the 0.8 build.

@rummik
Copy link

rummik commented Sep 17, 2013

@sorribas Travis doesn't seem to be catching this. It's just complaining about 0.10, 0.8 seems fine as far as it's concerned. Latest build (as of this message)

@sorribas
Copy link
Contributor

Oh. My bad. There was an error but apparently @visionmedia fixed it just today.

@tj
Copy link
Member

tj commented Sep 17, 2013

yeah we really need to stay away from hacky tests like that, they always end up being brittle, and not really testing the real framework anyway, I'm hugely against mocking when possible

@jonathanong
Copy link
Member

@rummik you should've opened an issue for the failing test earlier then! totally understandable if you didn't want to upgrade due to failing tests. although it's obvious that there's a bug, it's not a priority unless people speak up (especially since we know exactly what is failing).

@evjan opening an issue for the specific failing test is contributing (thanks for that). opening an issue saying tests are failing is not really helpful unless you're more specific or have suggestions. critiquing the culture, regardless of your definition, is not constructive, especially when there's in fact no team involved, just a single maintainer with very limited time.

@rummik
Copy link

rummik commented Sep 17, 2013

@jonathanong You mean the failing test that Travis notifies the project lead of every time the build fails? I thought that was Travis' job, not mine :P (I mean, that's why I use Travis, so I don't have to wait for someone to complain that I missed a failing test)

@jonathanong
Copy link
Member

@rummik if you have 500 repos like tj, you probably automatically archive them. they get annoying really quickly, and if you don't filter them out, you won't be able to find real emails.

@tj
Copy link
Member

tj commented Sep 17, 2013

haha I have 2600 unread emails :D, we'll get this fixed, no worries, but more help is better than more complaints, got the one fixed earlier, just need to look into the sendfile callback on linux machines and the prototype issue

@rummik
Copy link

rummik commented Sep 17, 2013

@jonathanong I'd hate to see the GitHub notifications for that many repos o.o Though, to cut down on Travis emails I'd probably set up filters to put failed build notifications in their own inbox, disable all build passing notifications, and disable builds on any branch but master (and possibly a handful of others)

Edit: And I'd also write a script to inject that info into all my repos' .travis.yml files, 'cause copy and paste with that many of them would take forever.

@jonathanong
Copy link
Member

@rummik what. i keep looking in travis profile settings but theres no options for email notifications. i'd much rather have it there than by changing the .travis.yml files.

@rummik
Copy link

rummik commented Sep 17, 2013

@jonathanong Unfortunately, there isn't a UI for it. I already looked. There's an issue open about it (travis-ci/travis-ci#1094), and one about making them branch specific (travis-ci/travis-ci#1405), but that's about all I saw from a quick glance

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

No branches or pull requests

5 participants