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

Split UI tests into e2e vs rest_api integration suites #5965

Merged
merged 1 commit into from Nov 25, 2015

Conversation

jwforres
Copy link
Member

TODO

  • delete the test project that is created using the Delete button on the settings page
  • update assets README

Implements https://trello.com/c/NFzYPctJ

@jwforres
Copy link
Member Author

@liggitt @fabianofranz @spadgett @benjaminapetersen first pass, just splits up the tests into two suites. if you have your api server running locally you can do

grunt test-integration --suite rest_api

and that suite can always be run, repeatedly

@jwforres
Copy link
Member Author

[test]

@jwforres
Copy link
Member Author

unrelated re[test]

@jwforres
Copy link
Member Author

unrelated [test]

@jwforres
Copy link
Member Author

[test]

@benjaminapetersen
Copy link
Contributor

I'm looking at integration/helpers.js and integration/suites/rest-api/project.js and wondering if we can sort all the helpers before they grow. Perhaps:

/integration 
  /helpers
   - setup.js
   - auth.js
   - navigate.js
   - input.js
   - project.js

Though it looks like most of the helpers at the top of project.js are only used in that block that is commented out at the bottom.

@benjaminapetersen
Copy link
Contributor

Thinking about urls as well, might be nice to have a utility to abstract these away/pack them into one file (something like routes).

// current
 h.goToPage('/project/' + project['name'] + '/browse/builds');
// possibly something like
navigate.to.project.builds(project.name);

@benjaminapetersen
Copy link
Contributor

Curious about /integration/e2e.js. Guessing we may eventually want:

/test
   /integration
   /e2e
   /spec (unit)

@jwforres
Copy link
Member Author

I debated this, the main reason I didnt was that I didn't want to have to
require a bunch of different files. How much do we think the number of
helpers will actually grow over time?

On Thu, Nov 19, 2015 at 4:18 PM, Ben Petersen notifications@github.com
wrote:

I'm looking at integration/helpers.js and
integration/suites/rest-api/project.js and wondering if we can sort all
the helpers before they grow. Perhaps:

/integration
/helpers

  • setup.js
  • auth.js
  • navigate.js
  • input.js
  • project.js

Though it looks like most of the helpers at the top of project.js are
only used in that block that is commented out at the bottom.


Reply to this email directly or view it on GitHub
#5965 (comment).

@benjaminapetersen
Copy link
Contributor

Thats a great Q. :)

@benjaminapetersen
Copy link
Contributor

Huge improvement already breaking that big e2e/test.js file down this much.

@spadgett
Copy link
Member

Last test error looks legitimate.

1)  authenticated e2e-user new project when creating a new project should delete a project
  - NoSuchElementError: No element found using locator: by.cssContainingText(".modal-dialog .btn", "Delete this project")

@jwforres
Copy link
Member Author

weird it was working locally

On Fri, Nov 20, 2015 at 8:46 AM, Sam Padgett notifications@github.com
wrote:

Last test error looks legitimate.

  1. authenticated e2e-user new project when creating a new project should delete a project
  • NoSuchElementError: No element found using locator: by.cssContainingText(".modal-dialog .btn", "Delete this project")


Reply to this email directly or view it on GitHub
#5965 (comment).

@jwforres
Copy link
Member Author

@benjaminapetersen for the integration vs e2e question. i'd be ok with that folder structure, but then it makes it weird to call the grunt task test-integration.

so we have grunt test which runs the spec tests, which seems to be a pretty standard naming convention for that task.

then we have all of the tests that require the api server to be started, a subset of those being the e2e tests which additionally require the e2e tests to have already run

@fabianofranz
Copy link
Member

Like @benjaminapetersen mentioned, when I worked on these tests I had a similar idea of abstracting url's to a central place. It would require a simple templating mechanism to replace project name and other variables in the url, e.g. /project/{{project-name}}/browse/builds. Do we have mustache or something similar included in the libraries?

@fabianofranz
Copy link
Member

Actually would not require, but would make it more readable. :)

@jwforres
Copy link
Member Author

we use URI.js for url templating

On Fri, Nov 20, 2015 at 9:36 AM, Fabiano Franz notifications@github.com
wrote:

Like @benjaminapetersen https://github.com/benjaminapetersen mentioned,
when I worked on these tests I had a similar idea of abstracting url's to a
central place. It would require a simple templating mechanism to replace
project name and other variables in the url, e.g.
/project/{{project-name}}/browse/builds. Do we have mustache or something
similar included in the libraries?


Reply to this email directly or view it on GitHub
#5965 (comment).

@jwforres
Copy link
Member Author

Ok so either way I have to add an npm dependency to make it accessible as a require in the protractor tests, would you guys rather include mustache as a generic template tool that we can use for other stuff besides URLs or stick with URI.js for url building. This is specifically for test cases, not in actual code.

@benjaminapetersen
Copy link
Contributor

I'm good with continuing with URI.js... the URITemplate constructor is pretty decent. I'm thinking something like:

// perhaps named path.js
return {
  to: {
    project: {
        create: new URITemplate('/project/' + projectName + '/create'),
        images: new URITemplate('/project/' + projectName + '/catalog/images')
        // etc
    }
  }
}
// used like
path.to.project.images(projectName);

@benjaminapetersen
Copy link
Contributor

Throwing this out there for some thoughts.

grunt test running the spec (unit) tests by default makes sense to me. I don't have an issue with these as grunt tasks:

    grunt test
    grunt test-e2e
    grunt test-integration

    grunt test-integration --suite rest_api

Should we flip-flop integration & e2? I'm thinking of integration as a few of our front end components together, but e2e as larger scale/api

Directory structure something like:

  • tests
    • unit
    • e2e
    • integration
      • project
        • builds
          • builds.spec.js
          • build.spec.js
          • config.spec.js
        • other siblings....

Thinking about suites. Perhaps we don't need a subdirectory `/suites', since a suite can be configured in the protractor-chrome.conf.js with arrays & blobs, letting us mix things up a bit:

suites: {
   // runs just the create test
   project_create: 'tests/e2e/project/create.Spec.js',
   // runs the 2 under pods
   project_pods: 'tests/e2e/project/pods/**/*.Spec.js',
   // runs the 3 under builds
   project_builds: 'tests/e2e/project/builds/**/*.Spec.js',
   // BUT this suite runs all the things...
   rest_api: [
      'tests/e2e/project/**/*Spec.js',
       // more stuff
   ]   
};

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to c9016f8

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7423/)

@jwforres
Copy link
Member Author

yay [merge]

@jwforres jwforres added the lgtm Indicates that a PR is ready to be merged. label Nov 24, 2015
@jwforres jwforres changed the title [WIP] Split UI tests into e2e vs rest_api integration suites Split UI tests into e2e vs rest_api integration suites Nov 24, 2015
@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/4147/) (Image: devenv-rhel7_2801)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to c9016f8

openshift-bot pushed a commit that referenced this pull request Nov 25, 2015
@openshift-bot openshift-bot merged commit 9c62ce5 into openshift:master Nov 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/web lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants