qpixel icon indicating copy to clipboard operation
qpixel copied to clipboard

Temporary branch for testing CI failures

Open trichoplax opened this issue 8 months ago • 22 comments

Not a real pull request.

This is just a way to run the CI system tests to try to understand why they are failing on #1548

trichoplax avatar Mar 23 '25 23:03 trichoplax

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 73.50%. Comparing base (1a59c4e) to head (98909da). :warning: Report is 2 commits behind head on develop.

:warning: Current head 98909da differs from pull request most recent head 9086e61

Please upload reports for the commit 9086e61 to get more accurate results.

Additional details and impacted files
Components Coverage Δ
controllers 68.67% <ø> (ø)
helpers 79.01% <ø> (ø)
jobs 48.57% <ø> (ø)
models 85.86% <ø> (ø)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Mar 23 '25 23:03 codecov[bot]

Just a quick test to check whether #1571 is enough to fix up the error (apparently, not)

Oaphi avatar Mar 25 '25 04:03 Oaphi

Now that it isn't just me looking at this branch, I should make explicit:

The tests added on this branch will never pass. They fail deliberately in order to force screenshots to reveal what the test users see. For anyone else following this, you can see the latest screenshots on the "Artifacts" tab of the CircleCI pages listed under "2 failing checks" on this page.

@Oaphi Thanks for the fixes and for checking if they helped here. I notice that this pull request only shows 1 file changed. Is what happened that you merged in the changes from 1571 and then force pushed back to how this branch was before that merge? I noticed that the test failures on the CI still start with the nil post_type problem, but is that just because this branch no longer has your fixes?

trichoplax avatar Mar 25 '25 13:03 trichoplax

When the underlying problem(s) are fixed, I would expect the screenshots to change from showing zero posts to showing 7 posts in the Main category and 1 post in the Meta category (7 posts present in the fixtures, and 1 post added to Meta by standard_user during a test).

trichoplax avatar Mar 25 '25 13:03 trichoplax

I've merged 0valt/seeds back in because I'd like to see the screenshots out of interest.

trichoplax avatar Mar 25 '25 14:03 trichoplax

I had forgotten that without my changes in #1548 there is a filter on the Main and Meta categories for standard_user, causing them to see zero posts. Therefore it is only basic_user who will see 7 posts in Main.

The screenshot for the test test_temporary_test_to_analyse_CI_failures_what_does_basic_user_see_in_main should show how many posts basic_user sees in the Main category. Before merging in 0valt/seeds it showed basic_user signed in, with 0 posts in Main. After merging in 0valt/seeds it shows standard_user signed in, with a red message "You are already signed in". This suggests that the test now starts with standard user signed in, so that the attempt to sign in basic_user fails.

Screenshot from test that should show basic_user signed in instead showing standard_user signed in and red message "You are already signed in"

trichoplax avatar Mar 25 '25 14:03 trichoplax

Now that it isn't just me looking at this branch, I should make explicit:

The tests added on this branch will never pass. They fail deliberately in order to force screenshots to reveal what the test users see. For anyone else following this, you can see the latest screenshots on the "Artifacts" tab of the CircleCI pages listed under "2 failing checks" on this page.

@Oaphi Thanks for the fixes and for checking if they helped here. I notice that this pull request only shows 1 file changed. Is what happened that you merged in the changes from 1571 and then force pushed back to how this branch was before that merge? I noticed that the test failures on the CI still start with the nil post_type problem, but is that just because this branch no longer has your fixes?

Yup, I, merged my changes in, waited for the run to fail, then force-pushed to remove the merge commit (planned to leave it up to you to revert but decided that cleaning by myself is a better option)

Oaphi avatar Mar 25 '25 14:03 Oaphi

Thanks for confirming. Now that I've merged your changes back in I can see that the nil post_type problem seems to be fixed (thank you!) and the screenshots suggest a different problem.

trichoplax avatar Mar 25 '25 14:03 trichoplax

Running the tests locally does not result in the problem of standard_user already being signed in for the basic_user test. The screenshot shows basic_user signed in as intended, but now with 0 posts showing in Main (rather than the previous 7 posts).

Previously:

  • CI showed 0 posts for basic_user in Main, and showed the nil post_types error message before running the tests
  • Locally showed 7 posts for basic_user in Main

Now:

  • CI shows standard_user already signed in so basic_user fails to sign in, and no nil post_types error message before running the tests
  • Locally shows 0 posts for basic_user in Main

trichoplax avatar Mar 25 '25 15:03 trichoplax

Also it seems like system test 3.1 and 3.2 differ in that only the 3.1 test has the "already logged in" error displayed if I am not mistaken?

Oaphi avatar Mar 25 '25 15:03 Oaphi

Also it seems like system test 3.1 and 3.2 differ in that only the 3.1 test has the "already logged in" error displayed if I am not mistaken?

Oh - thanks for checking. Previously Ruby 3.1 and 3.2 were only differing in how much of the nil post_type error was being displayed, and I didn't think to check both this time. Yes, 3.1 shows "You are already signed in" with standard_user signed in when that wasn't part of the basic_user test, while 3.2 shows the same as I'm seeing locally (where I'm running 3.1.6) - basic_user signed in with 0 posts in Main instead of the 7 present in the fixtures.

trichoplax avatar Mar 25 '25 15:03 trichoplax

I added a test to see what basic_user sees in Main, but this time with a log_out before attempting to log_in. This gives a screenshot of a plain white page (no header, no footer, completely blank). This happens locally (where previously basic_user could sign in but saw zero posts), on CI for 3.1 (where previously basic_user could not sign in due to standard_user being already signed in) and on CI for 3.2 (which previously looked identical to locally).

trichoplax avatar Mar 25 '25 15:03 trichoplax

After you added log_out on teardown, I still saw the blank white page for the test that tries to log_out before seeing what basic_user sees, for CI on both 3.1 and 3.2, and locally.

I also saw "You are already signed in" for "what_does_standard_user_see_in_main" on both CI 3.1 and locally, but not for CI 3.2. I'm not sure if this is a real difference between versions or intermittent behaviour.

trichoplax avatar Mar 25 '25 15:03 trichoplax

The tests run in a random order each time, so if behaviour is inconsistent it suggests there is still state being carried over from one test to the next.

trichoplax avatar Mar 25 '25 15:03 trichoplax

The tests run in a random order each time, so if behaviour is inconsistent it suggests there is still state being carried over from one test to the next.

There does seem to be state poisoning between tests, yeah. I went on a tangent to install FF in the container to be able to run system tests locally - now that I can test without waiting for the pipeline to run, it looks like the log_out fails with the same error as my teardown callback: Unable to find css ".header" in that test.

Oaphi avatar Mar 25 '25 15:03 Oaphi

After doing some cursory testing, from what I can see, one of the issues is that preemptive log_out doesn't work simply because it needs a page to be visited - the tests start in an empty state at least in terms of page visited (thankfully, as expected). You can confirm that by visiting a page (for example, new_user_session_url) and taking a screenshot.

It also doesn't look like "temporary test to analyse CI failures what does basic user see in main with log out first" starts with a poisoned state, at least with the latest commit (the visited page correctly shows that there is no signed in user).

Oaphi avatar Mar 25 '25 16:03 Oaphi

The screenshot for test_temporary_test_to_analyse_CI_failures_what_does_basic_user_see_in_main_with_log_out_first was still showing blank (just plain white) for CI and local.

After adding a page visit before trying to log out, it now shows a logged out page (on CI 3.1, 3.2, and locally). The test fails with Unable to find link "Sign out", suggesting it started out logged out rather than incorrectly signed in as standard_user as before. However, there is still something wrong as the communities switcher is open, suggesting a click was made not specified in the test.

Also, the signed out page shows 7 posts (as expected) locally, but 0 posts on CI (3.1 and 3.2).

Locally: 7 posts showing on a signed out Main page with the communities switcher open

CI (3.1 and 3.2): 0 posts showing on a signed out Main page with the communities switcher open

trichoplax avatar Mar 25 '25 19:03 trichoplax

There is also a new failure on CI 3.1 (not failing on CI 3.2 or locally). It's test_Creating_a_question_is_blocked_when_body_is_too_short and it says:

ActiveRecord::RecordNotFound: Couldn't find Post with 'id'=1015078639 [WHERE `posts`.`community_id` = ?]
    app/controllers/posts_controller.rb:652:in `set_scoped_post'

I don't know what is causing it or why it only shows up on CI and only for 3.1.

trichoplax avatar Mar 25 '25 19:03 trichoplax

The older test test_temporary_test_to_analyse_CI_failures_what_does_basic_user_see_in_main without trying to log out first now works and fails with a screenshot showing basic_user signed in. Locally it shows that basic_user sees 7 posts as expected, but on CI (3.1 and 3.2) basic_user sees 0 posts.

trichoplax avatar Mar 25 '25 19:03 trichoplax

Now that CircleCI is back up I have merged in develop and the tests now show basic_user sees zero posts in Main even locally (previously locally 7 were showing correctly).

trichoplax avatar Apr 01 '25 22:04 trichoplax

Now that CircleCI is back up I have merged in develop and the tests now show basic_user sees zero posts in Main even locally (previously locally 7 were showing correctly).

That's actually reassuring - now the result is consistent. May it be that the seeds now apply properly so we need to adjust accordingly?

Oaphi avatar Apr 01 '25 22:04 Oaphi

Even the signed out test "what does basic user see in main with log out first" shows zero posts in main after basic_user fails to log in, so it does look like the posts are simply not present rather than being a user visibility problem.

Yes I like the consistency. I don't know enough about how the fixtures work to understand why they aren't being picked up. I had previously thought that the fixtures were used before each individual test, but it now seems that's not the case and I don't know why.

trichoplax avatar Apr 01 '25 22:04 trichoplax