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

Travis build #50

Merged
merged 5 commits into from
Nov 28, 2018
Merged

Travis build #50

merged 5 commits into from
Nov 28, 2018

Conversation

oserban
Copy link
Member

@oserban oserban commented Nov 26, 2018

No description provided.

@senakafdo
Copy link
Member

@oserban, you have two broad options regarding the tests.

  1. Uncomment all of those test failures and encourage people to postpone writing tests even further.
  2. Do not include npm test in the Travis CI build until all apps have the their test cases written.

I personally would like to force people to write tests rather than by-passing this by passing all apps that do not have tests. This is obviously a personal choice, but I think it has its benefits. WDYT?

@oserban
Copy link
Member Author

oserban commented Nov 27, 2018

I don't think code shaming would work well in this case, because we already have broken build that has been ignored for the last few months. I can't push code that will break the build on every pr, because that means that whoever pushes the first charge on the app repo will have to implement all the tests. I would leave it like this and ask app authors to implement the tests when they are ready.

@senakafdo
Copy link
Member

Your point on the build not passing is something I can take, but equally, a build passing without test cases does not mean you have pushed code that is stable.

I would not, however, agree with the point, whoever pushes the first charge on the app repo will have to implement all the tests - as it will only break if you specifically enforce this on the Travis build. App authors can implement test cases at their own pace, but whoever decides to resolve #48 and subsequently confirm the stability of an OVE apps release will have to ensure all apps have test cases. That is what I'm specifically trying to enforce.

In other words, I'm saying, it is ok to tell the world that the test cases have not been executed on a build and therefore it is not stable rather than saying there were no test cases to run therefore it could be stable. I think the two statements are distinctively different.

@oserban
Copy link
Member Author

oserban commented Nov 27, 2018

I still believe that my proposal is better for the project because it allows us to add tests iteratively, for each app.

@jamesscottbrown
Copy link
Member

I still believe that my proposal is better for the project because it allows us to add tests iteratively, for each app.

Yes, it would clearly be unreasonable to require someone to create tests for all of the apps, rather than adding tests for one of them (which would be an improvement, but not resolve Issue #48).

In other words, I'm saying, it is ok to tell the world that the test cases have not been executed on a build and therefore it is not stable rather than saying there were no test cases to run therefore it could be stable. I think the two statements are distinctively different.

Yes, and I think the second statement is more correct: if something is untested, its status is unknown.

I think the correct statement to make is that as there are no automated tests unintended regressions may have been introduced.

I would avoid the use of stable in this context, as I don't think either commonly-use meaning is really relevant: OVE may be stable (or not) in the sense of changing slowly regardless of whether it has tests; and to have any confidence that it is stable in the sense of not crashing we would need to run it continuously for several days.

@senakafdo
Copy link
Member

senakafdo commented Nov 27, 2018

@jamesscottbrown - I have approved this PR based on @oserban's choice.

Unit testing does more than preventing unintended regressions (for instance see these docs from Microsoft)

And, you cannot simply justify that there were no test cases to run therefore it could be stable is proper. That is certainly a personal preference and generally not the norm in professional software development. A quality standard is meant to be met in whole and not in part. And, you would rather admit that something is unstable until it passes QA.

And, to add to that, stability and development activity are two broadly different things. Something that is rapidly developed may be stable as a result of it meeting a certain quality standard (confirmed either via manual or automated testing or by user acceptance). However, something that is not actively developed (i.e. very slow changes at the present compared to the past) does not always mean the software is stable (see http://www.engr.sjsu.edu/fayad/SoftwareStability/Andrsnw2.html).

The only exceptions are if you were on a pre-release branch or if it were a separate repository concerning a non-core component.

@jamesscottbrown
Copy link
Member

tl;dr: we should use a different word

And, to add to that, stability and development activity are two broadly different things. Something that is rapidly developed may be stable as a result of it meeting a certain quality standard (confirmed either via manual or automated testing or by user acceptance). However, something that is not actively developed (i.e. very slow changes at the present compared to the past) does not always mean the software is stable (see http://www.engr.sjsu.edu/fayad/SoftwareStability/Andrsnw2.html).

The article that you have linked does not actually define stability: the term stable is in practice used to refer to both speed of development and reliability (see e.g. https://bitdepth.thomasrutter.com/2010/04/02/stable-vs-stable-what-stable-means-in-software/), which I why I suggested using a more specific term that has only the intended meaning.

If you mean "has been tested" or "has passed QA" then say that, rather than "stable". Note that software may have both "passed QA" and be "unstable" (in the sense of frequently crashing in actual use) if something has been overlooked in the QA process.

The definition of stability in ISO 9126 is "The capability of the software product to avoid unexpected effects from modifications of the software": this definitely is improved by having regression tests, but saying that a particular version of the software is stable because it has been tested seems like a misapplication of the definition (which refers to the capability to avoid unexpected effects from modifications, not the absence of unexpected effects resulting from a specific modification).

@senakafdo
Copy link
Member

senakafdo commented Nov 27, 2018

@jamesscottbrown,

TLDR: There is nothing wrong with the word stable, if you understood the intended use of it, which is dependent on the context that it has been used.

I believe you have confused stability of software (which is typically what I meant) with what is meant by a stable release and reliability in production (as per your previous comment).

As you have quoted, ISO 9126 is clear on its definition of stability, which is The capability of the software product to avoid unexpected effects from modifications of the software. This is exactly why I said you cannot call a software to be stable if there exists no way of proving that it is stable. If you read broadly, this is specifically why developers are encouraged to write unit tests.

That said, I did not say that all software that passes QA is stable. I specifically said that it would be better to treat something as unstable if it never passed QA.

I'm not sure where we are heading with this conversation, but the original comment was precisely on the point that there is no point confirming that a build passed testing when there are no tests available. I also highlighted that the best possible resolution would be for all apps to ensure they have at least a minimum level of testing as a part of resolving #48.

@senakafdo senakafdo merged commit 2a66556 into master Nov 28, 2018
@senakafdo senakafdo deleted the travis-build branch November 28, 2018 20:21
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

Successfully merging this pull request may close these issues.

3 participants