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

Invalid example in ReusingStepDefinitions.md #65

Closed
eternalmatt opened this issue Aug 23, 2019 · 10 comments
Closed

Invalid example in ReusingStepDefinitions.md #65

eternalmatt opened this issue Aug 23, 2019 · 10 comments

Comments

@eternalmatt
Copy link

eternalmatt commented Aug 23, 2019

Please take look at ReusingStepDefinitions.md and notice that in the suggested example (second large code block) there is a variable myAccount. However this variable does not exist at this point in time. It seems this shared-steps was created as a result of hasty refactoring, and I see no way to link myAccount in the .steps file to the shared-steps.

My suggestion would be create a class in shared-steps which can return steps when new'd.

import { factory } from 'shared-steps'
const { givenIHaveXDollarsInMyBankAccount} = new factory(myAccount);
@boboz2
Copy link

boboz2 commented Sep 20, 2019

I also have a problem with the example ReusingStepDefinitions.md.
I follow the example but i have some error.

// shared-steps.js

export const givenIHaveXDollarsInMyBankAccount = given => {
    given(/I have \$(\d+) in my bank account/, balance => {
        myAccount.deposit(balance);
    });
};

export const thenMyBalanceShouldBe = then => {
    then(/my balance should be \$(\d+)/, balance => {
        expect(myAccount.balance).toBe(balance);
    });
};
// example.steps.js

import { loadFeature, defineFeature } from 'jest-cucumber';
import { thenMyBalanceShouldBe, givenIHaveXDollarsInMyBankAccount } from './shared-steps';

const feature = loadFeature('example.feature');

defineFeature(feature, test => {
  let myAccount;
		
  beforeEach(() => {
    myAccount = new BankAccount();
  });

  test('Making a deposit', ({ given, when, then }) => {
    givenIHaveXDollarsInMyBankAccount(given);

    when(/I deposit \$(\d+)/, deposit => {
      myAccount.deposit(deposit);
    });

    thenMyBalanceShouldBe(then);
  });
	
  test('Making a withdrawal', ({ given, when, then }) => {
    givenIHaveXDollarsInMyBankAccount(given);

    when(/I withdraw \$(\d+)/, withdrawal => {
      myAccount.withdraw(withdrawal);
    });		

    thenMyBalanceShouldBe(then);
  });
});
export class BankAccount {
  public balance: number = 0;

  public deposit(amount: number) {
      this.balance += amount;
  }

  public withdraw(amount: number) {
      this.balance -= amount;
  }
}

I have this error :
image

@Kiyozz
Copy link

Kiyozz commented Sep 23, 2019

Same as @boboz2 !

@bencompton
Copy link
Owner

Yes, that example is quite flawed. What we need is an easy way to share state between files, as suggested in #61.

@yellowbrickc
Copy link
Contributor

The state cannot really be "shared" because it belongs to the scenario but what about making the shared functions depending only on parameter and not on global states? It is a better coding style anyway ...

// shared-steps.js
export const givenIHaveXDollarsInMyBankAccount = (given, account) => {
    given(/I have \$(\d+) in my bank account/, balance => {
        account.deposit(balance);
    });
};

// example.steps.js
givenIHaveXDollarsInMyBankAccount(given, myAccount);

What do you think?

@Kiyozz
Copy link

Kiyozz commented Jan 19, 2020

The state cannot really be "shared" because it belongs to the scenario but what about making the shared functions depending only on parameter and not on global states? It is a better coding style anyway ...

// shared-steps.js
export const givenIHaveXDollarsInMyBankAccount = (given, account) => {
    given(/I have \$(\d+) in my bank account/, balance => {
        account.deposit(balance);
    });
};

// example.steps.js
givenIHaveXDollarsInMyBankAccount(given, myAccount);

What do you think?

The documentation needs to be updated then. This is real better. I use a class to shared my steps and states instead of functions.

@yellowbrickc
Copy link
Contributor

I can prepare a PR if you would like but it could take a few days. Should I or are you already working on it?

@Kiyozz
Copy link

Kiyozz commented Jan 19, 2020

I can’t work on it right now. I think you should do it 😉

@yellowbrickc
Copy link
Contributor

@bencompton @Kiyozz the PR should be made for the master or for 2.0.12 ?

@yellowbrickc
Copy link
Contributor

@bencompton @Kiyozz the PR should be made for the master or for 2.0.12 ?

I merge it to default (master), it is an inoffensiv merge anyway.

@bencompton
Copy link
Owner

Fixed in #76

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants