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

Don't report expected failures as failures to Travis; added #version:… #60

Merged
merged 1 commit into from
Feb 21, 2016

Conversation

theseion
Copy link
Collaborator

… property to the Metacello load spec

  • the test report would report run as failure even if only expected failures had actually failed
  • added #version: accessor to Metacello load spec. This allows one to specify a specific version that should be loaded when using a ConfigurationOf invocation.

@fniephaus fniephaus self-assigned this Feb 20, 2016
@fniephaus
Copy link
Member

Thanks! I may have time to go over this tomorrow evening.
Is this Squeak only?

@theseion
Copy link
Collaborator Author

The expected failures fix is for Squeak only. The addition of #version: concerns both Squeak and Pharo.

@fniephaus
Copy link
Member

@fniephaus
Copy link
Member

Re expected failures: since this can be quite confusing, I had already added these tests:
https://github.com/hpi-swa/smalltalkCI/tree/b63fb728df06b0b4f7e224554f4b917be0b62d21/tests/SmalltalkCI-Excluded-Tests.package

There's a test called testShouldPassUnexpectedly which is expected to fail, but it passes. So overall, this test should fail, right? With the current version, it actually fails. With your version, it does not. In fact, I think your version is the one I found in a recent Pharo image, so Pharo behaves exactly like your version. I would assume that my version is wrong then, but maybe this is a Pharo bug? If a test is expected to fail, it should not pass, right?

Since both of your changes are in one commit, I will need to wait until we have discussed this before I can merge this.

@fniephaus
Copy link
Member

This PR is also related to #47

@fniephaus
Copy link
Member

Re #60 (comment): now I get the problem! TestFailures were always written to the log, even if it they were expected. Also, your version is nicer, because it's less nested. Merging...

Lastly, Pharo's code is different and I'm afraid we found a bug...

@theseion
Copy link
Collaborator Author

What's the reason for the self halt.s?

Wow! Sorry about those. Simply forgot them.

In Pharo the philosophy of an expected failure seems to be that if a test fails then that is the expected outcome. In other words if the test doesn't fail then something has changed and the outcome is unexpected. A failure will then alert to a changed conditions.
I'm not saying that this is the best possible way to understand expected failures but it is a definition I have come to find convenient.
In Fuel we have a couple of test cases that we know are going to fail because they cover things that can't work (e.g. because of architectural constraints) and we use them for documentation. I don't want those tests to show up as failures because to a user they convey the impression that something is broken when really everything's fine.

So now I have all my beautifully green Fuel builds but Travis marks the (Squeak) builds as failures because the reporter exits with an error code.

If you look at the Squeak TestRunner, it will produce exactly that output by the way, exactly the same as in Pharo.

@theseion
Copy link
Collaborator Author

What's the bug in Pharo?

@fniephaus
Copy link
Member

Any way we can have a quick chat about this in Pharo's Slack or IRC?

@fniephaus fniephaus merged commit 9564df3 into hpi-swa:master Feb 21, 2016
@fniephaus
Copy link
Member

A test passing, even though it's marked as an expected failure, does not output anything in Pharo's HDTestReport. That's the bug I was talking about 😃

@dalehenrich
Copy link
Collaborator

I saw the same problems in the test runner when I ported it to GemStone. I fixed that behavior along with a couple of other things ... you know because these files are .st files and not a filetree repository we will never be able to share common code and we will continue to needlessly duplicate effort ...

@fniephaus
Copy link
Member

@dalehenrich yes, that is a problem. Unfortunately, Filetree does not ship with Squeak images. It'd be much easier if it did! I have just "gittified" the .st files (c14cbb2), so that I can see a normal diff and use GitHub's web editor. This makes things better, but not perfect.
If I think about it, I could just load Filetree in the build process for Squeak test images and then load smalltalkCI via Filetree in Squeak, Pharo and GemStone. That would make things even easier!

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