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

Fix tests and add lint and test workflows #20

Merged
merged 5 commits into from
May 7, 2024
Merged

Conversation

andrecsilva
Copy link
Contributor

@andrecsilva andrecsilva commented Apr 25, 2024

Fixes the current tests and adds an environment in which they can succeed.

  • Adds a test workflow.
  • Fix tests so they can actually succeed in the workflow.
  • Updates linting workflow to be a bit more strict.
  • Adds a urlopen version for the safe_requests api.

@andrecsilva andrecsilva changed the title Safe urlopen Fix tests and add lint and test workflows Apr 25, 2024
@andrecsilva andrecsilva marked this pull request as ready for review April 25, 2024 14:39
hooks:
- id: check-yaml
- id: check-json
- id: end-of-file-fixer
- id: trailing-whitespace
exclude: |
(?x)^(
src/core_codemods/docs/.*|
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you copied quite a bit of this and none of these locations exist in this repo, same for the other excludes, the mypy typing mocks that we may not be using

Copy link
Collaborator

@clavedeluna clavedeluna left a comment

Choose a reason for hiding this comment

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

It's hard to know what's part of the PR to "fix tests", what's urlopen, and what was just linting. Can you explain Fix tests so they can actually succeed in the workflow.? so it's more clear? what were the issues?

@andrecsilva
Copy link
Contributor Author

andrecsilva commented Apr 25, 2024

It's hard to know what's part of the PR to "fix tests", what's urlopen, and what was just linting. Can you explain Fix tests so they can actually succeed in the workflow.? so it's more clear? what were the issues?

The title is fairly descriptive of what this PR addresses.

Tests were failing and would fail in any machine for a myriad of reasons. Those range from not having netcat installed, not using a shell with brace expansion, permission issues, and many others. We didn't have any workflow for running tests for PRs, so those tests were merged into main while faulty. You're free to try to run them yourself and check the results.

This PR fixes those issues and adds a workflow to run those tests while preserving the intent of the original tests as much as possible. The workflow will help make sure we don't merge problematic tests/code again. I've also added a linting step to the workflow make sure the code is kept to a good standard in the future.

Before running into tests issues, my original with this branch was to add a version of safe_requests that uses urllib.request.urlopen instead of requests.get. We can't directly translate urlopen calls to requests.get due to some parameters of the former. This was included as well.

Copy link

Quality Gate Passed Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@andrecsilva andrecsilva merged commit 8adf7c6 into main May 7, 2024
4 checks passed
@andrecsilva andrecsilva deleted the safe_urlopen branch May 7, 2024 11:54
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.

2 participants