Skip to content

Commit d09e3fe

Browse files
Merge branch 'main' of github.com:NHSDigital/bcss-playwright into feature/BCSS-20436-calendar-util-markdown-doc
2 parents b600f73 + 98e88de commit d09e3fe

File tree

2 files changed

+164
-27
lines changed

2 files changed

+164
-27
lines changed

.vscode/settings.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
"cSpell.words": [
88
"addopts",
99
"autouse",
10+
"BCSS",
11+
"behaviour",
1012
"codegen",
1113
"customisable",
1214
"customised",

CONTRIBUTING.md

Lines changed: 162 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,175 @@
11
# Contributing To This Project
22

3-
With this project, we actively encourage anyone who may have any ideas or code that could make this repository better to contribute
4-
in any way they can.
3+
This document outlines the general guidance that should be applied when contributing new code to this project,
4+
to ensure that coding standards and principles remain consistent throughout the project.
55

6-
## How To Contribute
6+
## Table of Contents
77

8-
If you have an idea about something new that could be added, then please raise a
9-
[Feature Request via the Issues tab](https://github.com/nhs-england-tools/playwright-python-blueprint/issues/new/choose) for this
10-
repository. Even if you don't feel you have the technical ability to implement the request, please raise an issue regardless as
11-
the maintainers of this project will frequently review any requests and attempt to implement if deemed suitable for this blueprint.
8+
- [Contributing To This Project](#contributing-to-this-project)
9+
- [Table of Contents](#table-of-contents)
10+
- [General Principles](#general-principles)
11+
- [Use Playwright and pytest documentation as our standard](#use-playwright-and-pytest-documentation-as-our-standard)
12+
- [Proving tests work before raising a pull request](#proving-tests-work-before-raising-a-pull-request)
13+
- [Evidencing tests](#evidencing-tests)
14+
- [Example](#example)
15+
- [Coding Practices](#coding-practices)
16+
- [Tests](#tests)
17+
- [Docstring](#docstring)
18+
- [Example](#example-1)
19+
- [Page objects](#page-objects)
20+
- [Naming Conventions](#naming-conventions)
21+
- [Docstring](#docstring-1)
22+
- [Example](#example-2)
23+
- [Utilities](#utilities)
24+
- [Docstring](#docstring-2)
25+
- [Example](#example-3)
26+
- [Package management](#package-management)
27+
- [Last Reviewed](#last-reviewed)
1228

13-
If you have some code you think could be implemented, please raise a Feature Request and
14-
[create a fork of this repository](https://github.com/nhs-england-tools/playwright-python-blueprint/fork) to experiment and ensure
15-
that the change you want to push back works as intended.
29+
## General Principles
1630

17-
## Contribution Requirements
31+
### Use Playwright and pytest documentation as our standard
1832

19-
For any contributions to this project, the following requirements need to be met:
33+
When contributing to this project, we should be following the guidance outlined in the
34+
[Playwright](https://playwright.dev/python/docs/api/class-playwright) and
35+
[pytest](https://docs.pytest.org/en/stable/)
36+
documentation in the first instance to ensure our code remains as close to the recommended standard as possible.
37+
This will allow anyone contributing to this project to follow the code and when we use elements from either
38+
Playwright or pytest, easily reference the implementation from their documentation.
2039

21-
- You must be a member of the [NHS England Tools](https://github.com/nhs-england-tools) organisation on GitHub.
22-
- [Any commits must be signed](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits), so they show as verified once they reach GitHub. This checking serves as part of our CI/CD process, so unsigned commits will prevent a pull request from being merged.
23-
- For any utility methods that are added to this framework in the `utils` directory, the following applies:
24-
- Unit tests for the utility should be added to the `tests_utils` directory and need to be tagged with the `utils` mark.
25-
- Documentation for these classes and how to use any methods should also be added to the `docs/utilities-guide` directory.
26-
- Each method that is intended to be used as part of a class should have a correctly formatted docstring, to allow for developers using Intellisense within their IDE to understand what the code is intended to do.
27-
- All CI/CD checks will need to pass before any code is merged to the `main` branch - this includes ensuring appropriate formatting of code and documentation, security checks and that all unit and example tests pass.
40+
In the event we need to deviate away from this for any reason, we should clearly document the reasons why and explain
41+
this in any pull request raised.
2842

29-
## Things We Want
43+
### Proving tests work before raising a pull request
3044

31-
What we are particularly interested in is:
45+
When creating or modifying any code, if tests in the framework have been impacted by the change we should ensure that
46+
we execute the tests prior to raising a pull request (to ensure we are not asking for a code review for code that does
47+
not work as intended). This can either be done locally or via a pipeline/workflow.
3248

33-
- Any utility classes that can uniformly applied to any project. This may be something that's been created for your own project and by doing some minor abstraction any other teams working in a similar way could adopt this functionality.
34-
- Any development code that supports executing this project in a CI/CD fashion. This primarily covers any changes that support development principles outlined in the [Software Engineering Quality Framework](https://github.com/NHSDigital/software-engineering-quality-framework), and could include logic around how the test code is containerized.
35-
- Any changes that allow for test reporting in a consistent, reliable, maintainable and interesting format for varying stakeholders. This includes logic that expands on from the reporting we generate, such as example scripts for how to generate dashboards using the data we generate.
49+
### Evidencing tests
3650

37-
Examples:
51+
When we create new tests, or significantly change the functionality of a test, we should demonstrate these tests work
52+
by [recording the trace using Playwright](https://playwright.dev/python/docs/trace-viewer-intro) and attaching the
53+
generated trace file to the associated ticket within `Jira`.
3854

39-
- Say you've created a utility for generating test patients within your application. Any elements of this code that could be universally applied and other teams are likely to use (e.g. NHS number, patient name) we would want in this blueprint. If there's something business specific to your project that exists as part of this code (e.g. a unique reference number that only applies to your service), then we would advise removing that logic from any code before raising a pull request here.
40-
- If you do end up adding a utility class in this format in a more generic way to this project, you can subsequently [inherit the utility class](https://docs.python.org/3/tutorial/classes.html#inheritance) to include your additional business-specific requirements within your own version of the class.
55+
> NOTE: If the trace file exceeds the maximum file attachment size for `Jira`, we should
56+
> upload the file to a Confluence page instead and link this back to the ticket.
57+
58+
When we modify existing tests (including any page objects or utilities used by these tests) but the behaviour of the test
59+
has not fundamentally changed, we should upload the generated HTML report from the Playwright execution to the
60+
`Jira` ticket.
61+
62+
#### Example
63+
64+
We introduce a new test that covers the send a kit functionality for a single subject using `codegen` in the first instance.
65+
To demonstrate this test has worked as intended, we should turn tracing on and generate a trace file from Playwright and
66+
attach this to the ticket in `Jira`.
67+
68+
We then decide to refactor the test so that it uses a page object model for the send a kit page, but this does not change
69+
the behaviour of the test in any way (just makes the elements reusable). In this instance, we should upload the HTML report
70+
to the `Jira` ticket showing the test passing as the logic of the test has not changed in any way.
71+
72+
We then decide to create a utility that loops through several subjects at once and apply this to the previously created send
73+
a kit test. In this instance, we should turn tracing on again and generate a trace file from Playwright and attach this to the
74+
ticket in `Jira`, because the logic of the test has fundamentally changed.
75+
76+
## Coding Practices
77+
78+
### Tests
79+
80+
The following guidance applies to any files in the /tests directory.
81+
82+
#### Docstring
83+
84+
For any tests in the project, we should add a docstring that explains the test in a non-technical way, outlining the following
85+
points:
86+
87+
- The steps undertaken as part of the test
88+
- References to any applicable acceptance criteria this test covers
89+
90+
This information will be populated on the HTML report provided by the framework, allowing for non-technical stakeholders to
91+
understand the purpose of a test without specifically needing to view the code directly.
92+
93+
This should always be done using a [multi-line docstring](https://peps.python.org/pep-0257/#multi-line-docstrings), even if
94+
the test description is reasonably short.
95+
96+
##### Example
97+
98+
def test_example_scenario(page: Page) -> None:
99+
"""
100+
This test covers an example scenario whereby the user navigates to the subject search page,
101+
selects a subject who is 70 years old and validates their age is correctly displayed on the
102+
screening subject summary page.
103+
104+
This test is used for the following acceptance criteria:
105+
- BCSS-1234 (A/C 1)
106+
"""
107+
108+
### Page objects
109+
110+
The following guidance applies to any files in the /pages directory.
111+
112+
#### Naming Conventions
113+
114+
For any newly created page objects, we should apply the following logic:
115+
116+
- The filename should end with `_page` (Example: `send_a_kit_page.py`)
117+
- The class name should end with `Page` (Example: `SendAKitPage`)
118+
119+
#### Docstring
120+
121+
For any page objects in the project, we need to ensure for any class methods or functions we give a
122+
brief description of the intent of the function for the benefit of anyone reading the project or using
123+
intellisense. This can be done using a [single-line docstring](https://peps.python.org/pep-0257/#one-line-docstrings)
124+
where possible.
125+
126+
##### Example
127+
128+
class ExamplePage:
129+
130+
def __init__(page: Page) -> None:
131+
self.page = page
132+
133+
def click_on_locator(locator: Locator) -> None:
134+
"""Clicks on the provided locator."""
135+
locator.click()
136+
137+
def get_text_from_locator(locator: Locator) -> str:
138+
"""Returns the text from the locator as a string."""
139+
return locator.inner_text()
140+
141+
### Utilities
142+
143+
The following guidance applies to any files in the /utils directory.
144+
145+
#### Docstring
146+
147+
For any utilities added to the project, we should add docstrings that outline the functionality including
148+
any arguments and returns. These should be formatted as
149+
[multi-line docstrings](https://peps.python.org/pep-0257/#multi-line-docstrings) with a description, Args and
150+
Returns provided.
151+
152+
##### Example
153+
154+
def example_util(user: str) -> dict:
155+
"""
156+
Takes the user string and retrieves example information applicable to this user.
157+
158+
Args:
159+
user (str): The user details required, using the record key from users.json.
160+
161+
Returns:
162+
dict: A Python dictionary with the example details of the user requested.
163+
"""
164+
165+
### Package management
166+
167+
If we need to introduce a new Python package (via requirements.txt) into this project to allow for
168+
appropriate testing, we need to have critically reviewed the package and ensure the following:
169+
170+
- The package we intend to use is actively being maintained (e.g. has had recent updates or has a large active community behind it)
171+
- The package has appropriate documentation that will allow us to easily implement and maintain the dependency in our code
172+
173+
## Last Reviewed
174+
175+
This document was last reviewed on 01/05/2025.

0 commit comments

Comments
 (0)