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

GitHub Actions Migration #2451

Merged
merged 67 commits into from
Feb 3, 2023
Merged

GitHub Actions Migration #2451

merged 67 commits into from
Feb 3, 2023

Conversation

kopepasah
Copy link
Contributor

This pull request handles the migration from CircleCI to GitHub Actions.

@kopepasah kopepasah added the [Type] Enhancement Something new that adds functionality label Nov 24, 2022
@kopepasah kopepasah added this to the 2.27.0 milestone Nov 24, 2022
@kopepasah kopepasah self-assigned this Nov 24, 2022
kopepasah and others added 22 commits November 23, 2022 20:10
* Introduce validation-coding-standards.yml

* Change branch for testing

* Remove branch

* Add -

* Re-add branch

* update

* Yarn install before phpcs

* Composer install

* Composer lint

* Refactor composer install

* Tweak

* Tweak

* Test remove composer install

* Remove Install Composer steps

* Switch action trigger to pull_request

* yarn install --immutable

* actions/checkout@v3

* Updates
This allows us to wait until the data object is ready.
@kopepasah
Copy link
Contributor Author

@olafleur while we are still completing the deploy to wp.org portion of this task, please go ahead and start to review, as there are some big changes included.

.dev/tests/cypress/helpers.js Show resolved Hide resolved

disableGutenbergFeatures();
goTo( '/wp-admin/post-new.php?post_type=post' ).then( () => {
cy.wait( 2000 );
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why you still execute it in local now? In my tests, the wait was not needed, so that is a 2 seconds which is useless in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I could not get this working in GitHub Actions consistently with Cypress. Seems the code executes too fast, even though it is waiting on the data object. We would possibly add some additional check for the featureActive portion, but will suffice until we can complete the Playwright migration.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, no problem on the waiting on the CI (we dit 10s before, so 2s is an improvement). I was more wondering why you removed the if that made the tests not wait in local (as it is not needed).

.github/workflows/deploy-create-artifact.yml Outdated Show resolved Hide resolved
.github/workflows/test-e2e-cypress.yml Show resolved Hide resolved
if: ${{ inputs.theme == 'go' }}
run: |
cd $(wp-env install-path)/go
mkdir -p coblocks/icons
Copy link
Member

Choose a reason for hiding this comment

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

This custom SVG icon was not Go-specific, but CoBlocks specific (the Icon block), so it should be executed for any theme.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may need to circle back to this, then. As we will need to find an alternate way to use WP-ENV and add these into any theme within tests.

Copy link
Member

@olafleur-godaddy olafleur-godaddy Jan 31, 2023

Choose a reason for hiding this comment

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

Ok, but just to give a little context, we added this last year because we realized that when we added a custom icon (like specified here : https://github.com/godaddy-wordpress/coblocks/blob/master/docs/hooks/icons-block.md ) it would break the Icon block. So, the idea is just to make sure that we don't break that functionality again in CoBlocks by simulating the user adding a custom icon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I've got this covered in the latest change: 1bed6e1

@@ -2,7 +2,7 @@ const { defineConfig } = require( 'cypress' );

module.exports = defineConfig( {
chromeWebSecurity: false,
defaultCommandTimeout: 120000,
defaultCommandTimeout: 20000,
Copy link
Member

Choose a reason for hiding this comment

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

Oh, it's nice if we can reduce that time and not have flaky tests :)

@olafleur-godaddy
Copy link
Member

So, if I understand correctly, we don't test for variations of themes or browsers for the moment, right?

@kopepasah
Copy link
Contributor Author

So, if I understand correctly, we don't test for variations of themes or browsers for the moment, right?

Correct. We can add variations for themes, pretty quickly. I'll have to spend some time debugging why Firefox is not working in GitHub Actions, but let's tackle that in an alternate task.

@kopepasah kopepasah marked this pull request as ready for review February 1, 2023 21:03
@kopepasah kopepasah merged commit dd1e68e into master Feb 3, 2023
@kopepasah kopepasah deleted the feature/github-actions-migration branch February 3, 2023 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement Something new that adds functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants