Skip to content

Fixes #1603 - Updates CONTRIBUTING.md tests section for mock login.#1604

Merged
miketaylr merged 2 commits intowebcompat:masterfrom
brizental:issues/1603/1
Jun 21, 2017
Merged

Fixes #1603 - Updates CONTRIBUTING.md tests section for mock login.#1604
miketaylr merged 2 commits intowebcompat:masterfrom
brizental:issues/1603/1

Conversation

@brizental
Copy link
Copy Markdown
Contributor

@miketaylr
Copy link
Copy Markdown
Member

Looks good! I think we should also just recommend running functional tests always in test mode:

So we would want to update the following command a bit before your edits:

source env/bin/activate && python run.py -t

I'm not really sure why you would run them w/o fixtures or test mode.... wondering if we should document that it's possible. @karlcow?

@karlcow
Copy link
Copy Markdown
Member

karlcow commented Jun 21, 2017

@miketaylr

I'm not really sure why you would run them w/o fixtures or test mode.... wondering if we should document that it's possible. @karlcow?

We should definitely document any requirements.
if we look at the current doc, it says:

  • Running Tests

    • Functional Tests
    • Functional Tests using Fixture Data

Is there a reason why we ask to run the tests without the fixture data in the first part?

Start the application server:
source env/bin/activate && python run.py
In a separate terminal window or tab, run the tests:
node_modules/.bin/intern-runner config=tests/intern

I even wonder if node_modules/.bin/intern-runner config=tests/intern could simply becomes:

grunt functests which will start the server with the appropriate config.

@karlcow
Copy link
Copy Markdown
Member

karlcow commented Jun 21, 2017

Intern also comes with Grunt tasks so it can be quickly added to existing Grunt-based workflows, and is designed to work out-of-the-box with many popular continuous integration services like Jenkins and Travis CI.
https://theintern.github.io/intern/#what-is-intern

Grunt support is built into Intern. Install Intern and load the Grunt task into your Gruntfile using grunt.loadNpmTasks('intern'):
https://theintern.github.io/intern/#grunt

@miketaylr
Copy link
Copy Markdown
Member

OK, let's just make it required to run tests with fixtures, since that's what Travis does.

As for the grunt task, that's cool, but it can be a separate bug.

@miketaylr miketaylr self-requested a review June 21, 2017 16:40
Copy link
Copy Markdown
Member

@miketaylr miketaylr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brizental can you add the -t flag to command mentioned in my first comment?

Copy link
Copy Markdown
Member

@miketaylr miketaylr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants