Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/lint-js-and-ruby.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ jobs:
uses: actions/cache@v4
with:
path: vendor/bundle
key: package-app-gem-cache-${{ hashFiles('Gemfile.lock') }}-oldest
key: package-app-gem-cache-${{ hashFiles('Gemfile.lock') }}-lint
Copy link
Collaborator

Choose a reason for hiding this comment

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

Therefore, we should switch to using the newest spec/dummy gem cache.

Newest makes sense, but a separate -lint one doesn't to me.

Copy link
Contributor Author

@Judahmeek Judahmeek Jul 14, 2025

Choose a reason for hiding this comment

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

newest didn't work. That's why one of the commits is named "newest cache is still borked". It results in the gems being reinstalled just like the oldest cache key did.

The newest cache key works for other jobs, like Rspec, so I have no idea what is wrong.

All I know is that the lint key fixes the problem.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK.

- name: Install Node modules with Yarn for renderer package
run: |
yarn install --no-progress --no-emoji --frozen-lockfile
Expand All @@ -60,7 +60,7 @@ jobs:
uses: actions/cache@v4
with:
path: spec/dummy/vendor/bundle
key: dummy-app-gem-cache-${{ hashFiles('spec/dummy/Gemfile.lock') }}-oldest
key: dummy-app-gem-cache-${{ hashFiles('spec/dummy/Gemfile.lock') }}-lint
- name: Install Ruby Gems for dummy app
run: |
cd spec/dummy
Expand Down
10 changes: 10 additions & 0 deletions node_package/tests/jest.setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,13 @@ if (typeof window !== 'undefined') {
global.ReadableStream = ReadableStream;
global.ReadableStreamDefaultReader = ReadableStreamDefaultReader;
}

global.console.log('All calls to console have been disabled in jest.setup.js');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why? It's useful to see logs when running tests. Error/warning messages from React and other libraries in particular.

At least make an environment variable to disable this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The logs make it difficult to distinguish which test is failing and which console error is connected to the failure.

This change clears out all console output from the passing tests. Which makes seeing the output that actually matters a lot easier.

Happy to add an environment variable if you think it helps.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see why it would only clear console output from passing tests. If it does leave it in place for failing tests, great.


global.console = {
log: jest.fn(),
error: jest.fn(),
warn: jest.fn(),
info: jest.fn(),
debug: jest.fn(),
};
2 changes: 1 addition & 1 deletion node_package/tests/renderFunction.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@

describe('isRenderFunction', () => {
it('returns false for a ES5 React Component', () => {
const es5Component = createReactClass({

Check failure on line 12 in node_package/tests/renderFunction.test.jsx

View workflow job for this annotation

GitHub Actions / build

'es5Component' is assigned a value but never used
render() {
return <div>ES5 React Component</div>;
},
});

expect(isRenderFunction(es5Component)).toBe(false);
expect(true).toBe(false);
});

it('returns false for a ES6 React class', () => {
Expand Down
4 changes: 3 additions & 1 deletion spec/dummy/tests/react-on-rails.import.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import ReactOnRails from 'react-on-rails';

test('ReactOnRails', () => {
ReactOnRails.register({});
expect(() => {
ReactOnRails.register({});
}).not.toThrow();
});
4 changes: 3 additions & 1 deletion spec/dummy/tests/react-on-rails.require.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
const ReactOnRails = require('react-on-rails').default;

test('ReactOnRails', () => {
ReactOnRails.register({});
expect(() => {
ReactOnRails.register({});
}).not.toThrow();
});
Loading