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

[MISC] End-to-End Tests Refactoring #183

Merged
merged 48 commits into from
Aug 25, 2023
Merged

[MISC] End-to-End Tests Refactoring #183

merged 48 commits into from
Aug 25, 2023

Conversation

deusebio
Copy link
Collaborator

@deusebio deusebio commented Jul 4, 2023

In this PR, we remove the End-2-End tests from the current repository, and moved them to a Test repository, here. The reason for this was:

  1. Actions are inherently multi-repository, we should have a test repository where we can easily create pre-conditions (e.g. even with tags or branches, without the need to create the files), do commits, merge PRs on which we run actions from a given branch (where the process is implemented). Right now, we should keep on switching branches
  2. End-to-end tests interact heavily with the repository (by creating branches, tags, PR), and this would clutter the repository content. Beside, to prevent cluttering, one needs to be able to push force, delete, or other more dev features, that we don't want to enable for a clean, structured and maintanable source repository. Test repositories can have somewhat lighter rules when it comes to the rules of the repositories (e.g. force pushing branches, avoiding signed commits, rolling back). These are all sensible for the base project, but are a bit too restrictive for the test repositories, where we need to play with branches, PRs, etc
  3. We should avoid to clutter the main repos with tons of PRs & Triggered Workflows on the main repo as well as Discourse with cluttered content.

The idea discussed with @jdkandersson was to export the E2E tests into a separate repository. Of course this exposes us to the risk of merging features that are not working (altough pass unit and integration tests). The idea here would be to have edge and stable branches in this repository. Scheduled E2E tests workflow run against an edge branch in this repository, whereas actions in production should use stable branches. Promoting edge to stable should be a manual process done once we are comfortable a certain revision is good enough.

Tasks

I have now migrated most of the relevant code and business logic about end-to-end testing in the test repository, and I have also extended them to include raising PR and a conflict workflow (only hard conflict so far since the soft ones may go deprecated). This refactoring has however had impacts on this repository as

  1. some small fixes showed up during implementation of E2E tests
  2. we have structured the outputs of the process better (that are used in the E2E tests)
  3. Action inputs needed to be extended

There is a PR which shows some meaningful diff, on how that was achieved, alongside the tests workflow (against the branch of this PR), here and here. The code in the PR can have several improvements, but I believe it would be more critical to get the end-to-end tests up and running.

@jdkandersson
Copy link
Contributor

Should we make the test repository be in the canonical org?

action.yaml Show resolved Hide resolved
action.yaml Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
src/__init__.py Outdated Show resolved Hide resolved
src/repository.py Outdated Show resolved Hide resolved
tests/unit/test___init__.py Outdated Show resolved Hide resolved
@jdkandersson
Copy link
Contributor

It would be great if we can include some documentation on how to execute the new e2e tests and what the new branching strategy is (e.g., merge to edge first, run the e2e tests then merge to main or something like that

@deusebio
Copy link
Collaborator Author

Should we make the test repository be in the canonical org?

Yes. Let's address the PR and I will then create a brand new repository (I don't think the history we have there is very relevant)

jdkandersson
jdkandersson previously approved these changes Aug 22, 2023
merkata
merkata previously approved these changes Aug 24, 2023
yanksyoon
yanksyoon previously approved these changes Aug 24, 2023
Copy link
Contributor

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

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

LGTM, minor nitpicks, please feel free to resolve the conversations to merge!

action.yaml Outdated Show resolved Hide resolved
main.py Show resolved Hide resolved
main.py Show resolved Hide resolved
src/__init__.py Show resolved Hide resolved
Co-authored-by: Yanks Yoon <[email protected]>
@deusebio deusebio dismissed stale reviews from yanksyoon, merkata, and jdkandersson via 9a287c6 August 24, 2023 12:56
Co-authored-by: Yanks Yoon <[email protected]>
jdkandersson
jdkandersson previously approved these changes Aug 24, 2023
yanksyoon
yanksyoon previously approved these changes Aug 24, 2023
@deusebio deusebio dismissed stale reviews from yanksyoon and jdkandersson via 187d047 August 24, 2023 15:02
@github-actions
Copy link

Test coverage for 187d047

Name                      Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------------------------
src/__init__.py              71      0     28      0   100%
src/action.py               154      0     46      0   100%
src/check.py                 53      0     21      0   100%
src/clients.py               12      0      0      0   100%
src/commit.py                42      0     12      0   100%
src/constants.py              9      0      0      0   100%
src/content.py               50      0     10      0   100%
src/discourse.py            156      0     34      0   100%
src/docs_directory.py        33      0      8      0   100%
src/download.py              23      0      2      0   100%
src/exceptions.py            15      0      0      0   100%
src/index.py                128      0     48      0   100%
src/metadata.py              28      0     12      0   100%
src/migration.py             87      0     27      0   100%
src/navigation_table.py      65      0     20      0   100%
src/reconcile.py             87      0     38      0   100%
src/repository.py           282      0     88      0   100%
src/sort.py                  39      0     22      0   100%
src/types_.py               147      0     22      0   100%
---------------------------------------------------------------------
TOTAL                      1481      0    438      0   100%

Static code analysis report

Working... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:01
Run started:2023-08-24 15:21:45.880536

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 15407
  Total lines skipped (#nosec): 14
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

Copy link
Contributor

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

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

LGTM!

@deusebio deusebio merged commit 90fc71e into main Aug 25, 2023
23 checks passed
@deusebio deusebio deleted the wip-e2e-tests branch August 25, 2023 12:17
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.

4 participants