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

load 3.0.4 on top of 3.0.3 results in tests hitting WAAttributeNotFound #643

Closed
GoogleCodeExporter opened this issue Mar 25, 2015 · 13 comments

Comments

@GoogleCodeExporter
Copy link

To reproduce:

  1. unzip configurations.zip (attached)
  2. launch PharoCore1.1.1 image
  3. run the following in a workspace to preload the configurations (not that these configuration should not be committed to a public repository):

    Gofer new
    directory: '/export/foos1/users/dhenrich/pharo/monticello';
    package: 'ConfigurationOfGrease';
    package: 'ConfigurationOfKomHttpServer';
    package: 'ConfigurationOfOmniBrowser';
    package: 'ConfigurationOfRefactoringBrowser';
    package: 'ConfigurationOfSeaside30';
    package: 'ConfigurationOfSwazoo2';
    load.

  4. run the following in a workspace to load Seaside 3.0.3:

    ((Smalltalk at: #ConfigurationOfSeaside30) project version: '3.0.3') load: 'ALL'.

  5. save image
  6. run the following in a workspace to load Seaside 3.0.4:

    ((Smalltalk at: #ConfigurationOfSeaside30) project version: '3.0.4') load: 'ALL'.

  7. run the Seaside-Tests-Environment tests...you should get several tests with WAAttributeNotFound errors

If you skip step 4 and load Seaside 3.0.4. the tests run clean.

Presumably a class needs to be re-initialized after 3.0.4 is loaded ... not 
clear which one

Original issue reported on code.google.com by henrichs...@gmail.com on 11 Feb 2011 at 10:17

Attachments:

@GoogleCodeExporter
Copy link
Author

I've tracked this down to the fact that the WAFileHandlerConfiguration instance 
was initialized during the load of Seaside3.0.3 and 
WAFileHandlerConfiguration>>descirbeOn: has been significantly changed in 
3.0.4...including the addition of the #resourceBaseUrl attribute which is the 
cause of the WAAttributeNotFound. 

It's not clear to me exactly how the instances get created in the first place 
nor is it clear what the best way to reset the instance is ... 

It appears that WASystemConfiguration class>>clearAllDescriptions does the 
trick, but I'm not sure whether it is _safe_ to clearAllDescriptions in a 
production system. 

Original comment by henrichs...@gmail.com on 11 Feb 2011 at 11:04

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

The failure comes from search contexts that are not cleared correctly. When one 
adds a configuration option to an existing configuration description, the 
search contexts are not cleared. This is because a changed method doesn't 
affect the cached search contexts within the instances of the configurations.

First of all use the following as first-aid to the build process:
WAConfiguration allInstancesDo: [ :instance | instance clearSearchContexts ].
WAConfiguration allSubInstancesDo: [ :instance | instance clearSearchContexts ].

After that, we have two things to consider:
1) Clearing the search contexts is performed by:
WAConfiguration>>#clearSearchContexts
I think this should be added to the method: 
WASystemConfiguration>>#clearDescription
Any other places you think this should be added to?

2) Since adding a configuration setting means modifying a method 
(#describeOn:), the search contexts cannot be cleared without having a 
notification of some sort. Therefore, instead of trying to enforce each and 
every case of configuration "refresh", we can add to 
WAConfiguration>>#getSearchContextFor: a second lookup, which will not rely on 
the cache (in case there's a nil value on the cached search contexts). This 
way, we won't stumble upon a mismatch between configurations and their cache... 
Does this sound OK? 

Original comment by avish...@gmail.com on 11 Feb 2011 at 11:09

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

If you changed a #describeOn: before the context cache you had to do 
WASystemConfiguration class >> #clearAllDescriptions  or WASystemConfiguration 
class >> #clearAllDescriptions. This was a known limitation. I would lean 
towards a solution that flushes the context cache then the configurations are 
cleared.

Original comment by philippe...@gmail.com on 13 Feb 2011 at 11:53

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

You mean adding the call to clearSearchContexts to 
WASystemConfiguration>>#clearDescription ?
Notice that WASystemConfiguration class>>#clearAllDescriptions uses 
WASystemConfiguration>>#clearDescription.

Original comment by avish...@gmail.com on 13 Feb 2011 at 7:27

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Yes, but it doesn't do #allInstancesDo: / #allSubInstancesDo: which causes 
problems on GemStone and we can easily add it in a class side initialize.

Original comment by philippe...@gmail.com on 14 Feb 2011 at 9:32

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Never meant it to use #allInstancesDo: , I mentioned it because it is an 
immediate fix for clearing the search contexts.

Original comment by avish...@gmail.com on 14 Feb 2011 at 9:44

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Here's a proposed fix, including WASystemConfiguration>>#clearDescription

Original comment by avish...@gmail.com on 15 Feb 2011 at 11:56

  • Added labels: ****
  • Removed labels: ****

Attachments:

@GoogleCodeExporter
Copy link
Author

Looks OK. It's still like to see a class side #initialize in 
WASystemConfiguration or so that does a WASystemConfiguration >> 
#clearDescription for existing images.

Original comment by philippe...@gmail.com on 16 Feb 2011 at 6:16

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

OK, I merged this and added a class side #initialize in WASystemConfiguration 
that does a WASystemConfiguration >> #clearDescription for existing images.

I did the steps Dale wrote down and everything went fine.

I also took the 3.0.3 OneClick, ran the functional tests, then loaded the code 
and everything was still OK. This was not the case previously. I did the same 
thing again and before loading the updated code I changed some configuration 
settings on the default file handler.

Dale can you check again?

Original comment by philippe...@gmail.com on 19 Feb 2011 at 6:50

  • Changed state: Started
  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Philippe,

Will do ... I had a GemStone-Specific issue that I needed to address before 
proceeding with Seaside3.0.4 and I got through that today ...


Original comment by henrichs...@gmail.com on 23 Feb 2011 at 12:09

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

I merged Avi's fix from Seaside-Core-as.700.mcz  into the latest Seaside-Core 
and it the tests pass in an upgraded image (failed before integration)...

So I would say that Avi's fix is good and will copy his package to the 
repository along with a checkin of my merged fix...I'll mark this issue fixed.


Original comment by henrichs...@gmail.com on 28 Feb 2011 at 11:42

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Ah...merged in Philippe's two mcz files with related chnages:

Seaside-Core-pmm.707
Seaside-Core-pmm.706

need to retest ...

Original comment by henrichs...@gmail.com on 28 Feb 2011 at 11:46

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Works like a champ ... let's put this one to bed.

Name: ConfigurationOfSeaside30-dkh.301
Author: dkh
Time: 28 February 2011, 3:48:37 pm
UUID: 4c881600-b6aa-49dc-b927-d838ed8b7d08
Ancestors: ConfigurationOfSeaside30-dkh.300

Name: Seaside-Core-dkh.710
Author: dkh
Time: 28 February 2011, 3:48:04 pm
UUID: 84619027-b248-4750-b290-ad05f51ecc3e
Ancestors: Seaside-Core-pmm.709, Seaside-Core-pmm.707

Original comment by henrichs...@gmail.com on 1 Mar 2011 at 12:24

  • Changed state: Fixed
  • Added labels: ****
  • Removed labels: ****

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

No branches or pull requests

1 participant