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

92 - Execute e2e tests in GitHub action #131

Merged
merged 19 commits into from
Jul 25, 2023
Merged

92 - Execute e2e tests in GitHub action #131

merged 19 commits into from
Jul 25, 2023

Conversation

GPortas
Copy link
Contributor

@GPortas GPortas commented Jun 28, 2023

What this PR does / why we need it:

Re-enable the execution of E2E tests through GitHub actions.

Which issue(s) this PR closes:

Special notes for your reviewer:

The environment needs to know which branch of Dataverse to run in the containers.

For now, I've added the branch name as an environment variable within the action's yml file.

The variable has to be updated in the development of features to ensure that e2e executes the right Dataverse branch.

We may need to improve this mechanism in the future to make it more practical.

Suggestions on how to test this:

See how the action is executed in this PR.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

No

Is there a release notes update needed for this change?:

No

Additional documentation:

None

@GPortas GPortas changed the title 92 - Executing e2e tests in GitHub action 92 - Execute e2e tests in GitHub action Jun 28, 2023
@GPortas GPortas force-pushed the 92-e2e-environment branch 2 times, most recently from 0e702c7 to b17421d Compare June 29, 2023 12:03
@GPortas GPortas marked this pull request as ready for review June 29, 2023 12:42
@cmbz cmbz added the Size: 3 A percentage of a sprint. 2.1 hours. label Jul 5, 2023
@pdurbin pdurbin self-assigned this Jul 11, 2023
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

I'm excited it's working but let's talk about the 9588-datasets-api-extension branch being hard coded.

run: timeout 300s sh -c 'while ! docker logs dev_dataverse_bootstrap 2>&1 | grep -q "your instance has been configured"; do sleep 2; done'

- name: Run e2e tests
run: npm run test:e2e
Copy link
Member

Choose a reason for hiding this comment

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

Looks like it works! Great!

        Spec                                              Tests  Passing  Failing  Pending  Skipped  
  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ ✔  integration/datasets/DatasetJSDatav      00:24        4        4        -        -        - │
  │    erseRepository.spec.ts                                                                      │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  e2e/sections/dataset/Dataset.spec.t      00:24        6        6        -        -        - │
  │    sx                                                                                          │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  e2e/sections/hello-dataverse/HelloD      00:12        3        3        -        -        - │
  │    ataverse.spec.ts                                                                            │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  integration/info/infrastructure/rep      00:04        1        1        -        -        - │
  │    ositories/DataverseInfoJSDataverseR                                                         │
  │    epository.spec.ts                                                                           │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  integration/users/infrastructure/re      00:07        2        2        -        -        - │
  │    positories/UserJSDataverseRepositor                                                         │
  │    y.spec.ts                                                                                   │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘
    ✔  All specs passed!                        01:12       16       16        -        -        -  

@@ -2,41 +2,51 @@ name: test

on: push

env:
E2E_DATAVERSE_BRANCH: 9588-datasets-api-extension
Copy link
Member

Choose a reason for hiding this comment

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

Having 9588-datasets-api-extension hard coded here seems strange. Can we change this to develop after IQSS/dataverse#9693 has been merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that the integration tests must point to the Dataverse backend branch (image tag) used by the specific version of the js-dataverse package used, otherwise they could fail, not having the necessary Dataverse API changes.

We could set develop as you suggest (seems like a cleaner solution), but then we need to make sure to merge backend PRs first. This way the frontend PRs should wait (before accepting them) for the backend PRs to be merged for the e2e action to pass.

Choose a reason for hiding this comment

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

is there a way to pass repo and branch variables with the commit message?

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 could read the branch name of the commit message from the action via ${{ github.event.head_commit.message }} and detect if a branch name has been sent (e.g. surrounded the branch name by some text markers or using regex). In that case, we use that tag in the action. Otherwise, we use develop. What do you think?

Choose a reason for hiding this comment

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

It's what dataverse-ansible does (check for specified repo / branch, else assume defaults)

Copy link
Member

Choose a reason for hiding this comment

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

Back to the order of PRs being merged... recently we merged a frontend PR and a JS PR before we merged the backend PR they depend on. This is backwards. The backend PR should be merged first. Otherwise, the frontend and js work will have to be revisited if changes are required in the backend PR. I also mentioned this here: https://iqss.slack.com/archives/C010LA04BCG/p1686690116229119?thread_ts=1686603802.872139&cid=C010LA04BCG

@GPortas GPortas removed their assignment Jul 17, 2023
@pdurbin
Copy link
Member

pdurbin commented Jul 17, 2023

I'm seeing...

"npm ERR! code E400
npm ERR! 400 Bad Request - GET https://npm.pkg.github.com/IQSS/axe-playwright

npm ERR! A complete log of this run can be found in:
npm ERR! /home/runner/.npm/_logs/2023-07-17T13_54_07_297Z-debug-0.log
Error: Process completed with exit code 1."

... https://github.com/IQSS/dataverse-frontend/actions/runs/5576533857/jobs/10187960772

I'm going to rerun the e2e job.

@pdurbin
Copy link
Member

pdurbin commented Jul 17, 2023

After rerunning e2e it passed. It seems there might be some flakiness due to npm but let's keep an eye on it.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

I'm excited to see these e2e tests! Approved!

@pdurbin pdurbin removed their assignment Jul 17, 2023
@kcondon kcondon self-assigned this Jul 20, 2023
@kcondon
Copy link
Contributor

kcondon commented Jul 21, 2023

Tests pass, will merge once conflicts are resolved, @GPortas

@pdurbin
Copy link
Member

pdurbin commented Jul 21, 2023

@GPortas can you please resolve the conflicts in this PR?

@GPortas GPortas self-assigned this Jul 21, 2023
@GPortas
Copy link
Contributor Author

GPortas commented Jul 25, 2023

Done. After the merge with develop, I've had to add some fixes unrelated to the PR changes @kcondon

@GPortas GPortas removed their assignment Jul 25, 2023
@kcondon kcondon merged commit ea97942 into develop Jul 25, 2023
6 checks passed
@kcondon kcondon deleted the 92-e2e-environment branch July 25, 2023 13:27
jayanthkomarraju pushed a commit to jayanthkomarraju/dataverse-frontend that referenced this pull request May 31, 2024
92 - Execute e2e tests in GitHub action
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configure e2e test environment for GitHub actions
5 participants