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

Alsatian should warn when a test has no Expects executed #455

Open
jhm-ciberman opened this issue Feb 26, 2018 · 7 comments
Open

Alsatian should warn when a test has no Expects executed #455

jhm-ciberman opened this issue Feb 26, 2018 · 7 comments

Comments

@jhm-ciberman
Copy link
Contributor

Alsatian should warn when a test has no Expects executed.
In my code I have a lots of tests and I found test that werent executed. For example:

	@AsyncTest("should throw when loading an invalid template")
	public async loadFrom_invalid() {
		const loader = new TemplateLoader();
                // Note the commented "return"
		/* return */ loader.loadFrom(this.invalid.dir).then(() => {
			throw new Error("loadFrom() did not throw on invalid json");
		}).catch((e: Error) => {
			Expect(e).toBeDefined();
			Expect(e.message).toContain("Error loading Template");
		});
	}

The asyn test has no returns, so, the Expects are not executed. Alsatian should WARN when a test has no Expects (like other test runners), in this way those typing misstakes could be easily solved.

@Jameskmonger
Copy link
Contributor

This would be difficult with the current architecture, I think. It works currently by throwing an exception on failure. We could potentially achieve this by using an emitter pattern rather than relying on exceptions, but then we might lose stack traces...

@Jameskmonger
Copy link
Contributor

I do agree that it would be a useful feature, though, and something we should look to implement.

@cdibbs
Copy link

cdibbs commented Feb 27, 2018

From what I can tell, new Error().stack can yield a stack trace without actually throwing the error. Maybe that can help?

Also, I am working on a fluent Expect API for Alsatian, and will look at incorporating this, there. Certain parts of the API already safeguard against incomplete assertions at compile time.

For instance, when asserting on object properties:

Expect(sthg).with.properties({
  Prop0: actual => Expect(actual),
  Prop1: actual => Expect(actual).to,
  Prop2: actual => Expect(actual).to.equal(3)
});

Prop0 and Prop1 above will not compile, because Expect and to return types incompatible with the properties type definition.

However, outside of properties, Expect(sthg).to and Expect(sthg) will not produce compile-time errors (not sure they could), but maybe they could produce run-time errors. I'll think on this a bit, later.

@jamesadarich
Copy link
Member

Great suggestion!!!

I think this could tie in with allowing a test to fail multiple times #225

To enable that we'd need to know whether any expect calls failed, perhaps via metadata? As a result we should be able to get this in as well :)

@jamesadarich
Copy link
Member

With regards to empty expects raised by @cdibbs we should hopefully be able to detect that too if we collect the data about the expects run on that test

@pathurs
Copy link
Contributor

pathurs commented Jul 18, 2018

This could be solved in a similar fashion to my solution on #458

Proof Of Concept:

let expectations = [];

function Expect(anything) {
  expectations.push(anything);

  return { toBe: function() {} };
}

function Test(test) {
  expectations = [];

  test();

  if (expectations.length === 0) {
    console.error(`Test ${test.name} contained no expectations.`);
  }
}

Test(function test1() {
    Expect(1).toBe(1);
});

Test(function test2() {
});

Output:

Test test2 contained no expectations.

@jamesadarich
Copy link
Member

@pathurs absolutely this is roughly what we'd want for multiple failures (#255) all we'd need to do is add them to the metadata but instead of when we call Expect we should do it when a comparison has been made e.g. toBe so we can add that too the list to be executed all at once :)

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