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

Generic Sphinx-Needs JS test framework #1007

Closed
wants to merge 15 commits into from
Closed

Conversation

iSOLveIT
Copy link
Collaborator

@iSOLveIT iSOLveIT commented Sep 2, 2023

Running SN JS Test

Setup Cypress Locally

Enable Cypress Test in PyTest

  • First, create a test folder to store your Cypress JS test files (files should end with: *.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 the tests/js_test/js-test-sn-collapse-button.cy.js file as a reference.
  • In your Python test files, you must mark every JS-related test case with the marker - jstest and you also need to pass the spec_pattern key-value pair as part of the test_app fixture parameter. For example, your test case could look like the below:
# test_case.py

import pytest

@pytest.mark.parametrize(
    "test_app",
    [
        {
            "buildername": "html",
            "srcdir": "doc_test/variant_doc",
            "tags": ["tag_a"],
            "spec_pattern": "js_test/js-test-sn-collapse-button.cy.js",
        }
    ],
    indirect=True,
)
def test_case(test_app):
    ...

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 the specPattern.

  • After you have set the spec_pattern key-value pair as part of the test_app fixture parameter, you can call the app.test_js() in your Python test case to run the JS test for the spec_pattern you provided. For example, you can use it like below:
# tests_case.py

import pytest


@pytest.mark.jstest
@pytest.mark.parametrize(
    "test_app",
    [
        {
            "buildername": "html",
            "srcdir": "doc_test/variant_doc",
            "tags": ["tag_a"],
            "spec_pattern": "js_test/js-test-sn-collapse-button.cy.js"
        }
    ],
    indirect=True,
)
def test_collapse_button_in_docs(test_app):
    """Check if the Sphinx-Needs collapse button works in the provided documentation source."""
    app = test_app
    app.build()

    # Call `app.test_js()` to run the JS test for a particular specPattern
    js_test_result = app.test_js()

    # Check the return code and stdout
    assert js_test_result["returncode"] == 0
    assert "All specs passed!" in js_test_result["stdout"].decode("utf-8")

NB: app.test_js() will return a dictionary object containing the returncode, stdout, and stderr. Example:

{
    "returncode": 0,
    "stdout": "Test passed string",
    "stderr": "Errors encountered,
}

@iSOLveIT iSOLveIT requested a review from danwos September 2, 2023 09:28
Copy link
Member

@chrisjsewell chrisjsewell left a 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:

  1. Revert the changes to poetry.lock: no dependencies have changed, so this should not be changed.
  2. The description of how to run the tests should be part of the contributing documentation
  3. 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

@iSOLveIT
Copy link
Collaborator Author

iSOLveIT commented Sep 3, 2023

@chrisjsewell Thanks for the comments. The PR is actually a draft for @danwos to see if we would like to adopt this mechanism.

@chrisjsewell chrisjsewell marked this pull request as draft September 3, 2023 14:36
Copy link
Member

@danwos danwos left a 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 👍

tests/js_test/variant_doc/test-collapse-button.cy.js Outdated Show resolved Hide resolved
tests/test_variants.py Outdated Show resolved Hide resolved
tests/test_variants.py Outdated Show resolved Hide resolved
Copy link
Member

@danwos danwos left a 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.

docs/contributing.rst Outdated Show resolved Hide resolved
docs/contributing.rst Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Show resolved Hide resolved
tests/js_test/js-test-sn-collapse-button.cy.js Outdated Show resolved Hide resolved
tests/test_js_code.py Outdated Show resolved Hide resolved
iSOLveIT and others added 12 commits September 24, 2023 08:22
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.
@iSOLveIT
Copy link
Collaborator Author

Hi @everyone,
I will be closing this PR due to some commit and rebase issues.
You can find a resolved version of this PR in PR #1051.

@iSOLveIT iSOLveIT closed this Oct 24, 2023
@iSOLveIT iSOLveIT deleted the generic-SN-JStest branch October 24, 2023 15:38
danwos pushed a commit that referenced this pull request Nov 2, 2023
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.
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.

3 participants