-
Notifications
You must be signed in to change notification settings - Fork 71
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
Generic Sphinx-Needs JS test framework #1007
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @iSOLveIT, a few things to do initially:
- Revert the changes to poetry.lock: no dependencies have changed, so this should not be changed.
- The description of how to run the tests should be part of the contributing documentation
- Please move the test to a seperate file and it should also probably me marked (see https://pytest.org/en/7.4.x/example/markers.html) so that contributors can "opt-in" to running this test, since it brings more complexity and overhead than is required for most changes
FYI, I feel there might be a better way to integrate these tests into pytest, and also the vendoring of JS in sphinx-needs could be improved. But that doesn't need to be addressed here
@chrisjsewell Thanks for the comments. The PR is actually a draft for @danwos to see if we would like to adopt this mechanism. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work, and awesome documentation in the PR.
I agree with all points from @chrisjsewell, especially the PR description should be part of the contribute
page.
Also, the pytest-server handling should be done a little bit easier, see my comments.
But it all goes in the right direction and I'm happy to have this feature soon 👍
b844648
to
f42ad0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes. It getting better and better :)
Form the functional point of view, I only have some concerns with the parallel execution of tests and the hard-coded port number for cypress. Maybe there we can do some smarter port selection, if a port is already blocked.
The rest is only minor stuff.
This commit adds the ability to set an optional `error_message` key for each constraint in the `needs_constraint` configuration. This can contain Jinja placeholders, for needs data fields, and will be added to the need data, under the `constraints_error` field, if the constraint fails.
This commit is a step towards being able to generate the needs.json without running the Sphinx `BUILD` phase. Here we consolidate all functions that post-process the needs data, after it has been fully extracted from all documents and external sources. We remove the individual logic from within these functions, to check if the post-processing has already been done, and instead check before running all the functions. This refactor does not actually change the sphinx-needs process in any way, this will come later.
This commit refactors the functions that post-process needs data, to only get passed what they actually require (rather than just `app` or `env`). This makes it clearer what they are doing, and in all cases except `resolve_dynamic_values` effectively de-couples the post-processing from Sphinx, since you just need to pass them the needs data and needs configuration.
1c83dba
to
f1e86d8
Compare
The changes included are: - Updated conftest.py to enable JS testing using Cypress and PyTest. - Added ``make test-js`` command to Makefile. **NOTE:** This is an improved version of PR #1007 which is closed due to commit conflicts and rebase issues.
Running SN JS Test
Setup Cypress Locally
npm
package manager by runningnpm install cypress
. Read more from this link https://docs.cypress.io/guides/getting-started/installing-cypress#npm-install.npx cypress verify
. Read more from this link https://docs.cypress.io/guides/guides/command-line.Enable Cypress Test in PyTest
*.cy.js
). For each Cypress JS test file, you will need to write the Cypress JS test cases in the file. You can read more from the Cypress Docs. You can also read thetests/js_test/js-test-sn-collapse-button.cy.js
file as a reference.jstest
and you also need to pass thespec_pattern
key-value pair as part of thetest_app
fixture parameter. For example, your test case could look like the below:NB: The
spec_pattern
key-value pair is required to ensure Cypress locates your test files or folder. Visit this link for more info on how to set thespecPattern
.spec_pattern
key-value pair as part of thetest_app
fixture parameter, you can call theapp.test_js()
in your Python test case to run the JS test for thespec_pattern
you provided. For example, you can use it like below:NB:
app.test_js()
will return a dictionary object containing thereturncode
,stdout
, andstderr
. Example: