Add a new step: I should see “some text value” in the “Table Name” table #24

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@madmatt

madmatt commented Feb 4, 2014

The table name is generally specified by the <caption> element within the
<table> itself.

Add a new step: I should see “some text value” in the “Table Name” ta…
…ble.

The table name is generally specified by the <caption> element within the
<table> itself.
@madmatt

This comment has been minimized.

Show comment Hide comment
@madmatt

madmatt Feb 4, 2014

Arg, this appears to have pulled across another commit as well that should have been committed separately. It's also ready for merge, so this can be considered both. I'll squash it a bit later if that's easier to review.

madmatt commented Feb 4, 2014

Arg, this appears to have pulled across another commit as well that should have been committed separately. It's also ready for merge, so this can be considered both. I'll squash it a bit later if that's easier to review.

@chillu

This comment has been minimized.

Show comment Hide comment
@chillu

chillu Feb 9, 2014

Owner

Hmmm, this collides with @Then /^the "([^"]*)" table should contain "([^"]*)"$/ in CmsUiContext. I guess we could treat both grid fields and <table> as "tables" as far as the UI is concerned, right? In this case, could you consolidate the two methods and move them into BasicContext?

Owner

chillu commented Feb 9, 2014

Hmmm, this collides with @Then /^the "([^"]*)" table should contain "([^"]*)"$/ in CmsUiContext. I guess we could treat both grid fields and <table> as "tables" as far as the UI is concerned, right? In this case, could you consolidate the two methods and move them into BasicContext?

@chillu

This comment has been minimized.

Show comment Hide comment
@chillu

chillu May 12, 2014

Owner

@jeffreyguo Good idea, although its a bit unclear what this test actually does. Is row order important? Can you leave out columns? And we need to stick to "should contain" for simple view assertions, "has the following data" means "insert data into a database table". But anyway, different scope to this PR.

@madmatt Can you please change the PR to extend the existing "table should contain" step?

Owner

chillu commented May 12, 2014

@jeffreyguo Good idea, although its a bit unclear what this test actually does. Is row order important? Can you leave out columns? And we need to stick to "should contain" for simple view assertions, "has the following data" means "insert data into a database table". But anyway, different scope to this PR.

@madmatt Can you please change the PR to extend the existing "table should contain" step?

@jeffreyguo

This comment has been minimized.

Show comment Hide comment
@jeffreyguo

jeffreyguo May 13, 2014

@chillu Please ignore my comment above, the step definition is used for data input in a table, not for table data check. Sorry for my mistake.

@chillu Please ignore my comment above, the step definition is used for data input in a table, not for table data check. Sorry for my mistake.

@chillu

This comment has been minimized.

Show comment Hide comment
@chillu

chillu Jun 4, 2014

Owner

@jeffreyguo @srizzling Can one of you please look through the comments and figure out the most appropriate course of action? Ideally as a new pull request.

Owner

chillu commented Jun 4, 2014

@jeffreyguo @srizzling Can one of you please look through the comments and figure out the most appropriate course of action? Ideally as a new pull request.

@jeffreyguo

This comment has been minimized.

Show comment Hide comment
@jeffreyguo

jeffreyguo Jun 12, 2014

@chillu @madmatt consolidated in new pull request silverstripe-labs#50

@chillu @madmatt consolidated in new pull request silverstripe-labs#50

@chillu chillu closed this Jun 12, 2014

jeffreyguo pushed a commit to jeffreyguo/silverstripe-behat-extension that referenced this pull request Jan 12, 2016

Merge pull request #24 from IgorNadj/patch-1
ENH: allow specify database by filename
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment