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 #151

Merged
merged 10 commits into from
May 25, 2023
Merged

Fix tests #151

merged 10 commits into from
May 25, 2023

Conversation

fnesveda
Copy link
Member

I originally wanted to do a simple change to migrate the secrets used in the tests in CI to use organization-level secrets instead of repo-level ones. But when I was doing that change, I found out the template tests don't work at all, so I got into a very deep rabbit hole and I had to rewrite them basically from scratch.

This:

  • migrates the secret used in CI to be sourced from organization-level secrets
  • updates packages
  • removes some old dead code
  • adds linting to CI
  • rewrites template tests to actually work
    • installs dependencies for both Node and Python templates
    • runs the template and actually checks if the run succeeded or not
  • runs the tests in CI on Windows and Linux, with various Node and Python versions

There are still some issues:

  • I had to disable linting of templates, because their lint was broken, I will fix it later when there aren't changes being made to the templates, to prevent conflicts
  • Apify CLI returns exit code 0 even if the command fails, so it can't be relied on yet, I will fix it there and then update the tests here

@fnesveda fnesveda added bug Something isn't working. t-platform Issues with this label are in the ownership of the platform team. adhoc Ad-hoc unplanned task added during the sprint. labels May 24, 2023
@fnesveda fnesveda added this to the 64th sprint - Platform team milestone May 24, 2023
@fnesveda fnesveda self-assigned this May 24, 2023
Copy link
Member

@mnmkng mnmkng left a comment

Choose a reason for hiding this comment

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

Looks good. I trust that it works now 😄

Copy link
Member

@drobnikj drobnikj left a comment

Choose a reason for hiding this comment

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

👏

Copy link
Member

@jbartadev jbartadev left a comment

Choose a reason for hiding this comment

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

Nice, good job

@fnesveda fnesveda merged commit ae4a19a into master May 25, 2023
@fnesveda fnesveda deleted the fix/fix-tests branch May 25, 2023 09:00
@fnesveda fnesveda added the validated Issues that are resolved and their solutions fulfill the acceptance criteria. label Oct 9, 2023
@github-actions github-actions bot added the tested Temporary label used only programatically for some analytics. label Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adhoc Ad-hoc unplanned task added during the sprint. bug Something isn't working. t-platform Issues with this label are in the ownership of the platform team. tested Temporary label used only programatically for some analytics. validated Issues that are resolved and their solutions fulfill the acceptance criteria.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants