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

VSCode Devcontainer #1718

Merged

Conversation

pSchlarb
Copy link
Contributor

Addition of VSCode Devcontainer files.
Signed-off-by: Philipp Schlarb [email protected]

Signed-off-by: Philipp Schlarb <[email protected]>
@pSchlarb pSchlarb requested a review from a team as a code owner November 30, 2021 11:04
@sovbot
Copy link

sovbot commented Nov 30, 2021

Can one of the admins verify this patch?

python3-pip \
python3-nacl \
# rocksdb python wrapper
rocksdb=5.8.8 \
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use the latest version of rocksdb? v6.25.3?

Copy link
Contributor

Choose a reason for hiding this comment

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

I found the comment from line 28, the issue with plenum. Thanks. I understand the reason for version 5.8.8 now.

burdettadam
burdettadam previously approved these changes Dec 1, 2021
Copy link
Member

@WadeBarnes WadeBarnes left a comment

Choose a reason for hiding this comment

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

One small recommended change. Other than that I'm thinking we should have the pytest settings pre-configured so the development environment is more complete from the start. Thoughts?

.devcontainer/Dockerfile Show resolved Hide resolved
@pSchlarb
Copy link
Contributor Author

pSchlarb commented Dec 3, 2021

Other than that I'm thinking we should have the pytest settings pre-configured so the development environment is more complete from the start.

@WadeBarnes Do you mean providing the workspace settings.json for VScode ?
Its a good idea and already thought about it, but i am not quite sure.
If yes should it only be minimal settings for the tests or more customized?
Could it be problematic providing settings that don't work well with other settings?
To be honest i haven't that much experience with VSCode.

Co-authored-by: Wade Barnes <[email protected]>
Signed-off-by: Philipp Schlarb <[email protected]>
@pSchlarb pSchlarb force-pushed the Devcontainer-VSCODE branch from bc3ec03 to 50d50be Compare December 3, 2021 10:14
.devcontainer/Dockerfile Outdated Show resolved Hide resolved
@WadeBarnes
Copy link
Member

Other than that I'm thinking we should have the pytest settings pre-configured so the development environment is more complete from the start.

@WadeBarnes Do you mean providing the workspace settings.json for VScode ? Its a good idea and already thought about it, but i am not quite sure. If yes should it only be minimal settings for the tests or more customized? Could it be problematic providing settings that don't work well with other settings? To be honest i haven't that much experience with VSCode.

Yes, I was thinking some minimum settings for the tests, at least to start. It shouldn't be an issue since they would be meant to run in the dev container which is where we get the consistency for the development environment.

@pSchlarb
Copy link
Contributor Author

Hm I think we would have to create a pytest.ini since as i looked it isn't possible to set 2 working directories for the test discovery with the vscode settings.

Copy link
Member

@WadeBarnes WadeBarnes left a comment

Choose a reason for hiding this comment

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

One small recommendation.

I'm also looking into some issues running the tests in the container. Some tests "appear" to fail with a "Test result not found" error, when in fact they either pass or are skipped as expected. You can see the true summary in the Output - Python Test Log console. The issue appears to be a bug in the https://github.com/microsoft/vscode-python extension. There have already been some fixes for related issues.

// "forwardPorts": [],

// Use 'postCreateCommand' to run commands after the container is created.
"postCreateCommand": "pip install .[tests]",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"postCreateCommand": "pip install .[tests]",
"postCreateCommand": "pip install flake8==3.8.4 .[tests]",

Install flake8 otherwise there are Python (Output - Python) errors reported indicating flake8 is not installed.

@WadeBarnes WadeBarnes mentioned this pull request Dec 28, 2021
@WadeBarnes
Copy link
Member

WadeBarnes commented Dec 28, 2021

I'm also looking into some issues running the tests in the container. Some tests "appear" to fail with a "Test result not found" error, when in fact they either pass or are skipped as expected. You can see the true summary in the Output - Python Test Log console. The issue appears to be a bug in the https://github.com/microsoft/vscode-python extension. There have already been some fixes for related issues.

I've confirmed (through the Output - Python Test Log console) all of the tests pass or are skipped as expected. Some of the indy_node tests fail if they are all run at once, which we know happens and is one of the reasons we split up the tests in the GHA workflow(s) as well.

The tests affected by the "Test result not found" issue are marked with a red circle with a red dot in the middle rather than a red circle with a red x inside. All of the affected tests are parameterized tests, although in some cases there are some tests within the same group where the test results are found and others where the test results are not found. For example:
image

The "Test result not found" issue does not affect the ability to run the tests in debug mode and set breakpoints to step through the code.

@WadeBarnes
Copy link
Member

@pSchlarb, Could you also add some documentation to the top of the Dev Setup document about using the VSCode Devcontainer environment as the preferred method to create a development environment and get started with the code?

@WadeBarnes
Copy link
Member

The issues with the tests have been marked as a known issue here; #1729

@WadeBarnes
Copy link
Member

Request for documentation has been added as a separate issue here; #1728

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.

5 participants