Setup running e2e tests in Docker image by sfc-gh-gpijanski · Pull Request #70 · streamlit/component-template

sfc-gh-gpijanski · 2023-08-16T14:05:25Z

Each component's tests are run in a separate Docker image for environment isolation

dev.py

examples/RadioButton/e2e/test_radio_button.py

sfc-gh-pbelczyk · 2023-08-21T08:06:56Z

dev.py

-    """"Install all dependencies needed to run e2e tests for all examples and templates"""
+def cmd_e2e_build_images(args):
+    """Build docker images for each component e2e tests"""
    for project_dir in EXAMPLE_DIRECTORIES + TEMPLATE_DIRECTORIES:


Is it possible to aggregate all examples in one docker container? Setup docker takes some time and we probably only need separate dockers for template and template-reactless. Probably it can be done in later PR

Yes, it's possible. Let's discuss it with Kamil tomorrow, but I like the idea :)

Our goal is less than 15 minutes and we seem to be achieving this so there is no need to optimize. If I was optimizing something, I would put playwright in the docker image so that we don't have to install the browser multiple times, but that's not even needed now. Multiple images and multiple containers are not a problem for me as long as they are lightweight.

Ok, fine for me for now. Do you agree @sfc-gh-pbelczyk ?

sfc-gh-pbelczyk · 2023-08-21T08:23:16Z

dev.py

+        )
+        e2e_dir = next(project_dir.glob("**/e2e/"), None)
+        if e2e_dir:
+            mount_catalog = e2e_dir.parts[-2]


Not sure why, but also create CustomDataframe for me, even if there is no tests. I would leave it for now, as we will be testing this component in the future anyway, but it will be possible to check that we are not creating an unnecessary container once we have all the tests in place

It is possible that you had an empty catalog "e2e" in this component

Actually I don't have, but probably it's waste of time to debug it, since we want to have this tests in the future

I added another check for the catalog e2e if it is not empty

dev.py

sfc-gh-kbregula · 2023-08-22T08:59:43Z

Dockerfile

+ARG STREAMLIT_VERSION="latest"
+ENV E2E_STREAMLIT_VERSION=${STREAMLIT_VERSION}
+
+# hadolint ignore=DL3013


Do we have hadolint setup for this repo?

Copy-pasted :P I am removing that comment unless you think that we need hadolint. As for me, I think we can live without it

Can you create an issue in EPIC: https://snowflakecomputing.atlassian.net/browse/STREAMLIT-3913 ?

sfc-gh-kbregula · 2023-08-22T09:02:05Z

dev.py

+            run_verbose([
+                "docker",
+                "run",
+                "--tty",


Suggested change

"--tty",

"--tty",

"--rm",

Running containers with --rm flag is good for those containers that you use for very short while just to accomplish something, e.g., compile your application inside a container, or just testing something that it works, and then you are know it's a short lived container and you tell your Docker daemon that once it's done running, erase everything related to it and save the disk space.
https://stackoverflow.com/questions/49726272/what-is-the-rm-flag-doing

sfc-gh-kbregula · 2023-08-22T09:05:05Z

dev.py

+                "--volume", f"{e2e_dir.parent}/:/component/{mount_catalog}",
+                image_tag,
+                "/bin/sh", "-c",  # Run a shell command inside the container
+                f"cd /component/{mount_catalog} && "


Suggested change

"--volume", f"{e2e_dir.parent}/:/component/{mount_catalog}",

image_tag,

"/bin/sh", "-c", # Run a shell command inside the container

f"cd /component/{mount_catalog} && "

"--volume", f"{e2e_dir.parent}/:/component/",

image_tag,

"/bin/sh", "-c", # Run a shell command inside the container

f"cd /component/ && "

I don't think we need to create a separate directory here, and this could potentially cause a shell injection threat. In this case, there is no real threat, because the input is under our control, but it is still worth being careful.

sfc-gh-kbregula · 2023-08-22T09:06:40Z

dev.py

+                f"pip install dist/*.whl && "  # Install whl package
+                f"pip install -e .[devel] && "  # Install dependencies


Suggested change

f"pip install dist/*.whl && " # Install whl package

f"pip install -e .[devel] && " # Install dependencies

f"pip install dist/*.whl[devel] && " # Install whl package and dev dependencies

We don't want to use local source code to run tests.

Your solution doesn't work. But I managed to change it to this:

f"pip install /component/dist/*.whl && " # Install whl package f"pip install /component/[devel] && " # Install dev dependencies

What do you think about?

find /dist/ -name '*.whl' | xargs -I {} echo '{}[devel]' | xargs pip install

Works. Thanks

sfc-gh-kbregula · 2023-08-22T09:08:59Z

.github/workflows/ci.yaml

-    - name: Install python dependencies
-      run: ./dev.py install-python-deps
+    - name: Set up Docker Buildx
+      uses: docker/setup-buildx-action@v2


Please pin individual actions to a full-length commit SHA to make the action immutable. For details, see: https://snowflakecomputing.atlassian.net/wiki/spaces/CITS/pages/2420933452/Github+Actions+and+Runners+Security+Baseline+Requirements#Req-#10-Using-Third-Party-Actions
https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions

sfc-gh-kbregula · 2023-08-22T09:13:46Z

dev.py

+
+ARGUMENTS = {
+    "e2e-build-images": [
+        ("--streamlit-version", "latest", "Streamlit version for which tests will be run."),


Can you extract each argument to a new variable to reduce code duplication?

ARG_STREAMLIT_VERSION=.... ARG_PYTHON_VERSION=....

sfc-gh-kbregula · 2023-08-22T09:29:33Z

.github/workflows/ci.yaml


    - name: Run e2e tests
-      run: ./dev.py run-e2e
+      run: ./dev.py e2e-run-tests


We want to run E2E tests for only one version of NodeJS and one version of Python, but we should use different versions of Streamlit and The Streamlit Component Library.

I will add this in https://snowflakecomputing.atlassian.net/browse/STREAMLIT-3898

sfc-gh-kbregula · 2023-08-22T09:30:31Z

.github/workflows/ci.yaml

@@ -98,17 +98,14 @@ jobs:
        path: dist/*.whl


This action job is called build-examples-templates, but I think it does a little more now.

While working on e2e setup for component-template (streamlit/component-template#70) we extract example to separate file, so command for run example component is now invalid.

sfc-gh-kbregula · 2023-08-22T16:32:47Z

.github/workflows/ci.yaml


-    name: Examples + Templates / node-version=${{ matrix.node-version }}  / component_lib_version=${{ matrix.component_lib_version }}
+    env:
+      NODE_VERSION: ${{ matrix.node-version }}


Suggested change

NODE_VERSION: ${{ matrix.node-version }}

NODE_VERSION: ${{ matrix.node_version }}

sfc-gh-kbregula · 2023-08-22T16:34:45Z

.github/workflows/ci.yaml

-      run: ./dev.py install-wheel-packages
+    - name: Set up Docker Buildx
+      if: matrix.node_version == '19.x'
+      uses: docker/setup-buildx-action@7703e82fbced3d0c9eec08dff4429c023a5fd9a9


Can you add human-readable version in the comments?
https://github.blog/changelog/2022-10-31-dependabot-now-updates-comments-in-github-actions-workflows-referencing-action-versions/

While working on e2e setup for component-template (streamlit/component-template#70) we extract example to separate file, so command for run example component is now invalid.

sfc-gh-gpijanski marked this pull request as ready for review August 18, 2023 12:45

sfc-gh-gpijanski requested review from sfc-gh-kbregula and sfc-gh-pbelczyk August 18, 2023 12:47

sfc-gh-pbelczyk approved these changes Aug 21, 2023

View reviewed changes

sfc-gh-pbelczyk reviewed Aug 21, 2023

View reviewed changes

dev.py Outdated Show resolved Hide resolved

sfc-gh-kbregula reviewed Aug 22, 2023

View reviewed changes

sfc-gh-kbregula approved these changes Aug 22, 2023

View reviewed changes

sfc-gh-gpijanski added 2 commits August 22, 2023 15:34

Setup running e2e tests in Docker images

bc6dc39

CR fixes

a38f1bd

sfc-gh-gpijanski force-pushed the dockerize-e2e-tests branch from 3b03a96 to a38f1bd Compare August 22, 2023 13:37

sfc-gh-gpijanski added 4 commits August 22, 2023 16:19

Fix CI. Run e2e only on latest node js

0480f6f

Fix

d44c4e9

Fix

9cc925b

Fix

7fa5c84

sfc-gh-kbregula reviewed Aug 22, 2023

View reviewed changes

sfc-gh-kbregula approved these changes Aug 22, 2023

View reviewed changes

Fix after review

fdc9575

sfc-gh-gpijanski merged commit 3abafe8 into master Aug 23, 2023

sfc-gh-pbelczyk mentioned this pull request Aug 23, 2023

Update components-api.md streamlit/docs#789

Merged

2 tasks

		f"pip install dist/*.whl && " # Install whl package
		f"pip install -e .[devel] && " # Install dependencies

	f"pip install dist/*.whl && " # Install whl package
	f"pip install -e .[devel] && " # Install dependencies
	f"pip install dist/*.whl[devel] && " # Install whl package and dev dependencies

	NODE_VERSION: ${{ matrix.node-version }}
	NODE_VERSION: ${{ matrix.node_version }}

Conversation

sfc-gh-gpijanski commented Aug 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfc-gh-kbregula Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfc-gh-kbregula Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sfc-gh-gpijanski commented Aug 16, 2023 •

edited

Loading

sfc-gh-kbregula Aug 22, 2023 •

edited

Loading

sfc-gh-kbregula Aug 22, 2023 •

edited

Loading