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

Add acceptance tests #84

Merged
merged 40 commits into from
Mar 18, 2020
Merged

Add acceptance tests #84

merged 40 commits into from
Mar 18, 2020

Conversation

johnwatkins0
Copy link
Member

Description of the Change

Resolves #17 by adding and configuring WP Acceptance and setting up a snapshot for testing.
Resolves #19 by adding an initial set of acceptance tests covering post save functionality.

Alternate Designs

Further tests could be added to cover the settings page.

Benefits

Acceptance tests can now be added for new features and bug fixes.

Possible Drawbacks

Verification Process

Tests in the GH Actions CI process should all pass.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Changelog Entry

  • Added WP Acceptance tests

@johnwatkins0 johnwatkins0 mentioned this pull request Mar 8, 2020
6 tasks
@johnwatkins0 johnwatkins0 self-assigned this Mar 8, 2020
@johnwatkins0 johnwatkins0 added this to the 1.1.0 milestone Mar 8, 2020
- name: Run WP Acceptance
if: matrix.php-versions == '7.2'
run: |
if [ -n "${{ secrets.AWS_ACCESS_KEY }}" ]; then ./vendor/bin/wpsnapshots configure 10up --aws_key=${{ secrets.AWS_ACCESS_KEY }} --aws_secret=${{ secrets.SECRET_ACCESS_KEY }} --user_name=Travis [email protected]; fi
Copy link
Member Author

Choose a reason for hiding this comment

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

@jeffpaul We'll need AWS_ACCESS_KEY and SECRET_ACCESS_KEY added to the repo secrets for the WP Acceptance snapshot. I adapted this line from Distributor, which uses Travis and passes Travis user credentials, though it should work the same in the GH action.

@jeffpaul jeffpaul requested a review from dinhtungdu March 8, 2020 18:38
@jeffpaul jeffpaul modified the milestones: 1.1.0, 1.0.2 Mar 8, 2020
@jeffpaul jeffpaul mentioned this pull request Mar 8, 2020
17 tasks
@johnwatkins0
Copy link
Member Author

@dinhtungdu This is ready for review. Took me 1,000 commits to get the WP Acceptance tests to pass 😫

composer.json Outdated
@@ -14,18 +14,28 @@
"role": "Developer"
}
],
"repositories": [
Copy link
Member Author

Choose a reason for hiding this comment

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

I will create a follow-up issue for this when this PR is merged. We'll need to switch back to the main WP Acceptance repo after Felipe's PR is merged.

Copy link
Member

Choose a reason for hiding this comment

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

@johnwatkins0 FYI that WPA has been updated from Felipe's PR.

@johnwatkins0 johnwatkins0 changed the title WIP: Acceptance tests Add acceptance tests Mar 12, 2020
@@ -42,3 +43,8 @@ jobs:
run: |
composer run setup-local-tests
composer test
- name: Run WP Acceptance
if: matrix.php-versions == '7.3'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we run tests on 7.2 too?

@johnwatkins0
Copy link
Member Author

@jeffpaul @dinhtungdu I'm having a lot of trouble with the GitHub action for this. After getting Acceptance tests to pass, the action is now failing on PHPUnit because mysql doesn't appear to be available in the container as of yesterday. I don't know what changed. I've done some searching in the GH forums and so far no one else is having this problem.

I propose:
(1) @dinhtungdu take a look at the most recent failing test action and the test.yml workflow and see if you see anything I might be missing.
(2) If no solution presents itself, we switch to Travis.

@jeffpaul
Copy link
Member

@johnwatkins0 last year we had an odd issue on ClassifAI and Travis and had to add this line to get things working again: https://github.com/10up/classifai/blob/978d79c9c6d1573a65bbac2db787366125a6f94f/.travis.yml#L27

It looks like you've got something similar in https://github.com/10up/autoshare-for-twitter/blob/feature/wp-acceptance-tests/.github/workflows/test.yml so maybe something's off in the way we're referencing mysql in that YML file?

@johnwatkins0
Copy link
Member Author

johnwatkins0 commented Mar 13, 2020

@jeffpaul Yeah, last night I found myself back in that Slack conversation about the missing mysql in the Travis config, so several of my recent failed test runs here are me trying different things to get mysql set up in the Actions environment without success. Before switching to Travis I do want to take a little more time to try to resolve it, but I also don't want this to block 1.0.2 if I can't figure it out quickly.

.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
johnwatkins0 and others added 2 commits March 13, 2020 16:17
composer.json Outdated Show resolved Hide resolved
johnwatkins0 and others added 2 commits March 13, 2020 16:42
@jeffpaul
Copy link
Member

@johnwatkins0 anything else planned on this PR or are we good to merge this in an have some fun new tests?

@johnwatkins0
Copy link
Member Author

johnwatkins0 commented Mar 17, 2020

@jeffpaul Yes, it's good to merge. I removed a couple of my more complicated acceptance tests in my last commit, specifically:

  1. A test that checked the autoshare box, saved the draft, unchecked the autoshare box, published the post, and verified no tweet was sent
  2. A test that saved a draft with the autoshare box unchecked, checked the box on the following pageload, published the post, and verified that the tweet was sent

Both of these tests were failing about 25% of the time. This is a little worrying, but I have not been able to replicate the failures in manual testing. If there is a bug in those scenarios, I think it's one that requires super-fast fingers for a human to replicate. Maybe we could merge this in and create a new issue for WP Acceptance tests for those two scenarios.

@jeffpaul jeffpaul mentioned this pull request Mar 18, 2020
2 tasks
@jeffpaul jeffpaul merged commit 2b5d88d into develop Mar 18, 2020
@jeffpaul jeffpaul deleted the feature/wp-acceptance-tests branch June 30, 2020 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for wp-acceptance [4 hrs.] Add and configure wp-acceptance [4 hr.]
4 participants