JulianNorton / weather-10kb-wxkb

Weather forecast that's high performance and accessible
https://wxkb.juliannorton.com
GNU General Public License v2.0
152 stars 49 forks source link

Automated accessibilty testing with tenon.io (#146) #167

Closed Tardog closed 6 years ago

Tardog commented 6 years ago

As discussed, the test uses the tenon.io API key provided by an environment variable (TENON_API_KEY). If the API key is undefined or empty, the test will be skipped – this will prevent test failures during development for contributors who don’t have a tenon.io key. For deployment, the environment must be set up to allow the tests to run.

As it turned out, the existing npm packages for tenon.io integration either a) lack the means to specify all options recognized by the API, or b) require an URL to test against. So I wrote a new module for our specific needs. It’s rather small, not much more than a request wrapper with a bit of parameter validation, and doesn’t introduce any additional dependencies. Tests for the module are also included in this PR.

I also made a few changes for more convenient development:

Let’s see if the tenon.io setup works with our CircleCI project!

@JulianNorton Before you can run the test successfully with your API key, please make sure you have a project in your tenon.io account with the correct ID ("'weather-10kb" is defined in the test, but please feel free to change this if you already created a project ID for the app).

Tardog commented 6 years ago

Checks are finished. There are still a few issues.

  1. Failing tests: This is due to Google API quota being exceeded (see logs). Re-run tomorrow?
  2. The accessibility test was skipped because the TENON_API_KEY environment variable is missing.
  3. PR check via Codacy: There are a lot of alerts because "strings must use double quote", but throughout the app we use single quotes. It would probably make more sense to change Codacy rules to match the existing code style.
JulianNorton commented 6 years ago

Hey @Tardog, glad to see you again!

  1. Google API shouldn't be hitting quota. We're well under the limits. screen shot 2017-10-02 at 11 21 58 am

  2. The tenon_api_key is in the project settings. Not sure why it's failing. screen shot 2017-10-02 at 11 27 31 am

  3. I've disabled the quote style in check Codacy. I didn't see an option to select single quotes. screen shot 2017-10-02 at 11 25 48 am

Anything I can do to help?

Tardog commented 6 years ago

@JulianNorton I love contributing here, so I’ll always come back when life doesn’t get in the way. :)

  1. There is something funky going on with the tests on CircleCI. The error messages for forecast-related tests fluctuate between these two: wxkb.io | Error: Status is OVER_QUERY_LIMIT. You have exceeded your daily request quota for this API. We recommend registering for a key at the Google Developers Console: https://console.developers.google.com/apis/credentials?project=_ wxkb.io | "Forecast cannot be retrieved. Response: 403 Forbidden

  2. Is there any way for you to see a build log with all private environment variables, to determine if they have been correctly set? It’s strange that previous builds from only two days ago pass the tests, yet now they are failing for both current PRs.

  3. Disabling the check is, in my opinion, not a long-term solution. I’ve logged into Codacy to see if I can find something else. And I did – there’s a carefully hidden "Details" button for each setting. It only appears if you hover somewhere over the text (but NOT the checkbox): codacy-quote-settings

Editing this to avoid double posting: So I’ve set up CircleCI with my fork of weather-10kb. Before adding the API keys in the app, tests failed with the same 403 error message we see in the recent builds of the main project. After I added the keys to the environment, all tests passed. Yes, even the one checking tenon.io. That still doesn’t tell us why the same setup fails on the main repo, however. :(

/second edit: On second thought, it makes perfect sense. The environment variables are set on a per-project basis. Anyone making a pull request from a fork (as opposed to using a branch on the main repository as a base) doesn’t get the environment variables set for your account at CircleCI. Thus, the tests fail until that user links their Github account with CircleCI and sets up their own API keys.

JulianNorton commented 6 years ago
  1. Builds pass when I run them, I think you're right about hiding the environmental variables!

  2. Quote Style: I've re-enabled quote checks and specified single. The screenshot was a big help in finding where it was hidden.

Thanks @Tardog!