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

Run ActiveRecord -> ActiveJob integration tests in CI #42492

Merged
merged 1 commit into from
Jun 15, 2021

Conversation

ghiculescu
Copy link
Member

In #42452 (comment), @smt116 pointed out that activerecord's Active Job integration tests are skipped in CI.

This PR moves them into the test/cases directory, so they are run. Originally I was going to add a new CI task but given they run pretty quickly I don't think that's worth the overhead.

@ghiculescu
Copy link
Member Author

image
confirmed relevant tests are no longer skipped 👍

@kamipo kamipo merged commit 8580855 into rails:main Jun 15, 2021
@ghiculescu ghiculescu deleted the run-activejob-tests branch June 15, 2021 14:50
@rafaelfranca
Copy link
Member

This PR is wrong. The Active Job tests on Active Record should not run int he same process as the regular Active Record tests.

@rafaelfranca
Copy link
Member

Reverted

@ghiculescu
Copy link
Member Author

@rafaelfranca should they run as a separate process? I can do a PR for that instead. Currently I don't think they are running at all.

@rafaelfranca
Copy link
Member

They should be in a separated process, in the same way that the actionview:integration:active_record run.

@rafaelfranca
Copy link
Member

It seems we only need to change the Rakefile like this: https://github.com/rails/rails/blob/main/actionview/Rakefile#L15

@rafaelfranca
Copy link
Member

We also need to run the performance tests for encryption.

@ghiculescu
Copy link
Member Author

Cool. I can have a go at that. Out of curiosity, what's wrong with the approach in this PR?

@rafaelfranca
Copy link
Member

It will make the Active Job integration with Active Record to be loaded in the others Active Record tests, causing a leak depending in the orders the tests are run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants